checkmate icon indicating copy to clipboard operation
checkmate copied to clipboard

Signal specialized error condition on assertion failure

Open mllg opened this issue 8 years ago • 5 comments

As suggested by @gaborcsardi.

mllg avatar Jun 30 '16 19:06 mllg

I was thinking whether we should put the error class definitions in a separate package. You might define assertionError, but other packages might want to use this error type in the future (yes, I am optimistic!).

So how about creating a package called conditions that has nothing but the error types? We can look at other languages and then adapt to R.

gaborcsardi avatar Jun 30 '16 20:06 gaborcsardi

Something like this: https://github.com/mllg/conditions ?

I've used some of the names from python exceptions (https://docs.python.org/2/library/exceptions.html), but not all seem to fit and R probably demands for more exception classes, e.g. for length and missingness. So this is more or less a very incomplete first draft.

Unfortunately, throwing conditions from C seems to be not possible at the moment, at least the API does not offer anything useful.

mllg avatar Jul 06 '16 12:07 mllg

YEah, something like that.

I think I would make it even more bare, and not assume anything about how the conditions are thrown. Just define them, lookup_error = condition(...), and then people could just say stop(lookup_error), as they please. If this is possible at all.

I don't think it is a big difference to what you did, but I think that if we want this package to be very generic, then it is best to make as few assumptions as possible.

gaborcsardi avatar Jul 06 '16 12:07 gaborcsardi

I've just pushed a second draft where only un-signaled conditions are returned.

Do you think that throwing generic errors without a customized error messages is the way to go? It might encourage to generate non-informative conditions.

I thought about adding some sugar to allow something like this:

stop(lookup_error + "Failed to lookup variable 'x'")

However, I will probably never use just the object lookup_error without setting a message, therefore I can set it directly via a constructor. What do you think?

mllg avatar Jul 06 '16 13:07 mllg

That's a good point, yes, custom messages should be allowed and encouraged. But we can still avoid signalling the condition. So I think what you have now is quite good.

Once we have some good examples in checkmate or elsewhere, I'll write to people to take a look. httr already defines some custom errors for various HTTP errors: 404, 403, etc. The DBI packages should definitely throw custom error types. There are probably other good candidates.

gaborcsardi avatar Jul 06 '16 14:07 gaborcsardi