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

[Gunnar Wrobel]

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.

Cheers,

Gunnar

-- 
Core Developer
The Horde Project

e: wrobel at horde.org
t: +49 700 6245 0000
w: http://www.horde.org

pgp: 9703 43BE
tweets: http://twitter.com/pardus_de
blog: http://log.pardus.de