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
The basic idea is good, but of course, some details need to be sorted out. As I explained in another response, I really think that we should have derived exceptions for many of the "kind"s so that they can be explicitly caught. And of course, in some cases, extra information can be put into the subclasses (e.g. UTFException contains extra data specific to it), so having derived exceptions is very valuable. Having the kinds is great, but I don't think that that should preclude having derived classes which can be explicitly caught, thereby allowing users to choose whether they want to catch the base type (and possibly use the kind field, possibly not) or to catch the derived types when they want error handling specific to the errors that those types represent. Using only kind fields rather than derived exception types makes it harder to use try-catch blocks to separate out error handling code like they were designed to do. 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. 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. 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. 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. - Jonathan M Davis