[dev] Dealing with Exceptions (was: Re: [commits] Horde branch master updated. f2b35e73225d8ec7487ef393dc610583fe2b1ece)

[Jan Schneider]

Zitat von Gunnar Wrobel <wrobel at horde.org>:

> Hi Jan,
>
> we already briefly discussed Exceptions on IRC and decided to  
> continue on the mailing (has been a while back, please forgive me  
> for the rather long delay on the topic).
>
>
> Zitat von Jan Schneider <jan at horde.org>:
>
>> The branch "master" has been updated.
>> The following is a summary of the commits.
>>
>> from: 1e943c0937d592233379d8cac82b89f80861b11c
>>
>> 43b8c54 Fix package name.
>> 7390733 Here too.
>> e627e65 Fix directory name, remove outdated tests.
>> f2b35e7 Consistently extend exception classes from  
>> Horde_Exception_Prior. CS, cleanup.
>
> Why should there be any need for a low level framework package to  
> extend from Horde_Exception_Prior?
>
> On IRC you mentioned that
>
> 1) this should be done so that re-throwing Exceptions can be done on  
> both 5.2 and 5.3 in a decent way (using the third argument of the  
> Horde_Exception constructor, named "previous")
>
> 2) it is convenient to allow throwing anything as the first argument  
> at the constructor (which is what Horde_Exception_Prior does on top  
> of Horde_Exception)
>
> 3) "this has to be consistent"
>
> I have been looking at the Kolab_Format package the last week and I  
> must say that I see no reason for it to extend from  
> Horde_Exception_Prior.
>
> It does not re-throw any exceptions as it is at the base of the  
> hierarchy. So Kolab_Format has no specific need to extend from  
> Horde_Exception. It would not use any of its functionality.
>
> The package also does not require any convenience methods when  
> constructing new exceptions. The only exception parameter is usually  
> just the message string. So Kolab_Format has no specific need to  
> extend from Horde_Exception_Prior either. It would not use any of  
> its functionality.
>
> This leaves "consistency" which is what I'd like to understand  
> first. My assumption is that this means that it should be consistent  
> on the application level. So that we can rely on catching  
> "Horde_Exception_Prior" and be certain that we can catch anything  
> coming from the underlying framework that way.
>
> If that is what you refer to with "this has to be consistent" then I  
> agree to the point that we need to know what to catch on the  
> application level. A generic "catch (Exception $e)" might not be a  
> good thing. But I would say that this still does not require all  
> framework libraries to extend from the same exception.
>
> Kolab_Format is at the base of the hierarchy. So it should not care  
> or know about the application level. Throwing a  
> Horde_Exception_Prior because some upper level might need it seems  
> like a problematic design choice to me. Kolab_Format should care  
> about its own business and ensure that it handles its exceptions in  
> a decent way.
>
> In case there is another layer on top using Kolab_Format it should  
> definitely ensure that it is able to deal with all Exceptions that  
> might be thrown from Kolab_Format. This would refer to Kolab_Storage  
> for example which is the main consumer for Kolab_Format. It should  
> always only throw its own type of exceptions. If it would run into  
> situations where a Horde_Kolab_Format_Exception bubbles to the  
> surface I would consider this to be a bug of Kolab_Storage.
>
> Now Kolab_Storage could decide to extend from Horde_Exception_Prior  
> or it could happen in an exception class of a package even further  
> up in the chain. Nearer to the application level.
>
> Kolab_Format is a good example to me because it is definitely used  
> stand-alone outside of any Horde context. And I don't see any reason  
> to force other consumers to pull in the Horde_Exception package if  
> that is not required.
>
> An additional remark: We should of course rename  
> Horde_Exception_Prior because I chose a real bad name for that  
> class. May "Horde_Exception_Base" or something like that would be  
> more appropriate.

First of all, I still think it makes sense to have a common base  
exception class for all horde code, so that consumers (whether those  
are horde applications or external code using some horde library) can  
implement some catch-all for any exceptions thrown by horde code. I  
see that it makes sense to throw special purpose PHP/SPL extensions  
though, and we actually do this, which is inconsistent again. But  
that's not the inconsistency I care about right now.

What I mean with consistency is that all horde exceptions should  
behave the same way. You may know that Horde_Kolab_Format_Exception is  
never going to wrap another extension/php error/pear error. But I  
don't. We have so many libraries, each using its own exception class,  
and I don't want do look up each exception's class definition to see  
how I have to use it, when I'm going to fix/code something in some  
arbitrary library. I want to be sure that any Horde exception class is  
working the same, so I can wrap it around any of the errors/exceptions  
that might occur during PHP development.

This helps a lot during development and it doesn't add much overhead  
either. All library exceptions should extend a common horde exception  
anyway, and whether this is Horde_Exception or Horde_Exception_Prior  
doesn't really matter.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/