DIP33: A standard exception hierarchy

[Lars T. Kyllingstad]

On Monday, 1 April 2013 at 23:03:56 UTC, Jonathan M Davis wrote:
> On Monday, April 01, 2013 13:08:15 Lars T. Kyllingstad wrote:
>> It's time to clean up this mess.
>> 
>> http://wiki.dlang.org/DIP33
>
> Another concern I have is InvalidArgumentError. There _are_ 
> cases where it
> makes sense for invalid arguments to be an error, but there are 
> also plenty of
> cases where it should be an exception (TimeException is 
> frequently used in
> that way), so we may or may not want an 
> InvalidArgumentException, but if you
> do that, you run the risk of making it too easy to confuse the 
> two, thereby
> causing nasty bugs. And most of the cases where 
> InvalidArgumentError makes
> sense could simply be an assertion in an in contract, so I 
> don't know that
> it's really needed or even particularly useful. In general, I 
> think that
> having a variety of Exception types is valuable, because you 
> catch exceptions
> based on their type, but with Errors, you're really not 
> supposed to catch
> them, so having different Error types is of questionable value. 
> That doesn't
> mean that we shouldn't ever do it, but they need a very good 
> reason to exist
> given the relative lack of value that they add.

I definitely don't think we need an IllegalArgumentException.  
IMO, passing illegal arguments is 1) a simple programming error, 
in which case it should be an Error, or 2) something the 
programmer can not avoid, in which case it requires a better 
description of why things went wrong than just "illegal 
argument".  "File not found", for example.

I didn't really consider contracts when I wrote the DIP, and of 
course there will always be the problem of "should this be a 
contract or a normal input check?"  The problem with contracts, 
though, is that they go away in release mode, which is certainly 
not safe if that is your error handling mechanism.


> Also, if you're suggesting that these be _all_ of the exception 
> types in
> Phobos, I don't agree. I think that there's definite value in 
> having specific
> exception types for specific sections of code (e.g. 
> TimeException for time-
> related code or CSVException in std.csv). It's just that they 
> should be put in
> a proper place in the hierarchy so that users of the library 
> can choose to
> catch either the base class or the derived class depending on 
> how specific
> their error handling needs to be and on whatever else their 
> code is doing. We
> _do_ want to move away from simply declaring module-specific 
> exception types,
> but sometimes modules _should_ have specific exception types.

There may of course be, and probably are, a need for more 
exceptions than what I've listed in the DIP.  The idea was to 
make a pattern, a system, to which more exceptions can be added 
if strictly necessary.  I do think, however, that we should try 
to keep the number at a minimum, and that we should NOT create 
new classes for every little detail that can go wrong.


> The focus needs to be on creating a hierarchy that aids in 
> error handling, so
> what exceptions we have should be based on what types of things 
> it makes sense
> to catch in order to handle those errors specifically rather 
> than them being
> treated as a general error, or even a general error of a 
> specific category.
> Having a solid hierarchy is great and very much needed, but I 
> fear that your
> DIP is too focused on getting rid of exception types rather 
> than shifting them
> into their proper place in the exception hierarchy. In some 
> cases, that _does_
> mean getting rid of exception types, but I think that on the 
> whole, it's more
> of a case of creating new base classes for existing exceptions 
> so that we have
> key base classes in the hierarchy. The DIP focuses on those 
> base classes but
> seems to want to get rid of the rest, and I think that that's a 
> mistake.

It aims to get rid of the ones that don't add any value.

> One more thing that I would point out is that your definition of
> DocParseException won't fly. file and line already exist in 
> Exception with
> different meanings, so you'd need different names for them in 
> DocParseException.

True.

Lars