smbache / ensurer

Ensure values are as expected at runtime
Other
84 stars 4 forks source link

Adding common checks #4

Closed gaborcsardi closed 10 years ago

gaborcsardi commented 10 years ago

Is this planned? E.g. ensure_string_scalar, ensure_file_exists, ensure_dir_exists, etc? Or the idea is that I can just add to my package whatever I need?

smbache commented 10 years ago

Initially this was the intention, but as I though about it I realized that the list would probably be ever-growing, and it might open up a can of worms. So I think I might keep it as simple as possible...

Also I typically find that the conditions needed are often very specific...

But this is not set in stone; have actually given it some thought :)

gaborcsardi commented 10 years ago

Yeah, it is a tough decision, and there might not be a perfect solution. How about having another package for common checks? I like it, that the package is clean, but I also don't want to duplicate the common checks in all my packages.

EDIT: I would be happy to create such a package, and most probably will, if you don't want to do it.

smbache commented 10 years ago

Do you have a list of what you would consider reusable/standard checks?

gaborcsardi commented 10 years ago

A reasonable list is here: https://github.com/hadley/assertthat#new-assertions assertthat is actually not bad, but the checks return a logical, and not the value itself, so you can't use them in pipes.

Another thing would be checking an enum, e.g.

f <- function(mode = c("foo", "bar", "foobar") {
  ensure_enum(mode) %>% ...
  ...
}

although this is almost the same as match.arg, but then we could have the same kind of error messages.

Others:

But you are right that it is hard to decide. E.g. I often need ensure_url nowadays, and I am not sure if that should be built in.

gaborcsardi commented 10 years ago

One solution to this is http://editorconfig.org/ Of course RStudio would need to support it....

EDIT: sorry, wrong issue.....

smbache commented 10 years ago

Would a solution be that you made the package, and let it evolve, then maybe it will be more clear if they are better suited in ensurer itself?

gaborcsardi commented 10 years ago

We can try, although I will probably put in more stuff than you would, and then you will not want to merge. :)

Btw. the assertive package is also not bad, at least it has a lot of assertions built in, but it also has a lot of other boilerplate stuff, and I am not quite comfortable with the syntax. Anyway, I'll create an ensurethat package. (Btw. suggestions for a better name are welcome. :)