Exceptions

[Jelmer Schreuder]

As promised ;-)

One of my pet peeves has been for a while to throw meaningfull exceptions. But I myself have been one of the worst bad-guys in this. Exceptions thrown by the Cache class are all Cache_Exceptions but have a simple filterable message, that is of course very wrong. If they should be filtereable, they should be different kinds of exceptions.

Instead of Cache_Exception('expired') and Cache_Exception('not found') it should be CacheExpired() and CacheNotFound, that would allow people to handle different exceptions differently (in this case CacheExpired should extend CacheNotFound, it's a special case of notfound and both should also be catcheable at the same time).

I've improved this for the Orm by creating different Exceptions for different conditions in the packages/orm/classes/exceptions.php file. But then I ran into a little bit of trouble as the exceptions for the observers really didn't belong there.

I want to make a couple of points here:

1. we should review all current exceptions thrown and see where specialized exceptions are in order.

2. the new setup should be well thought out and have some specialized superextensions that are extended by the specialized exceptions. The inheritance structure should also be well thought out, like in my cache examples above to allow catching some similar at once while also allowing specialized catching.

3. I think we should keep specialized exceptions with the file of the class or superclass to which they belong - otherwise we'd add a shitload of exception files which have little more than a class definition. This doesn't need to be considered to break our 1-class-per-file rule as exceptions are a special type of class. (with superclass I mean for example that Cache exceptions should be kept in core/classes/cache.php instead of the storage drivers)

4. We'd have to decide on how to write them: snake cased or camelcased. I've always written exceptions camelcased as I consider them PHP language constructs, but it would probably be prefereable to make these use underscores as well.

I don't think this would necessarily break any backwards compatibility, so this might be considered for 1.1.

Jelmer

[Jelmer Schreuder]

To add to this, I was just reading through a blogpost mentioned at phpdeveloper.org: http://codeutopia.net/blog/2011/05/06/how-to-use-built-in-spl-exception-classes-for-better-error-handling/

We should probably also start using these SPL exceptions in some cases.

[Dan Horrigan]

Yes to all.

We should use SPL Exceptions where we can, other wise use custom exceptions.

Custom Exceptions should use CamelCase as they are to be treated the same as a standard PHP class, which is always CamelCase.

Examples of where SPL Exceptions and Custom Exceptions should be used:

- If an item is not found...in any class (whether it be a Cache item or a Session item) should throw OutOfBoundsException.

- For numeric array keys not found a OutOfRangeException should be thrown

- Use BadMethodCallException where applicable in __call and __callStatic methods

Basically where it makes sense to use SPL, do it.  If it doesn't make sense, throw your own.

Exception classes should be at the TOP of the super class' file (e.g. In the cache.php file for all cache classes).  This will ensure people see them and they aren't lost.

Good call Jelmer.  I aways forget about the SPL Exceptions.

Dan

[Jelmer Schreuder]

I went ahead and made some massive changes to the Exceptions used in the core, thought it would be good to have this change in for a large part for RC3. Far from finished though.