Re: PEAR_Exception in CVS

Hi Tomas, Tomas V.V.Cox wrote:

public function getCauseMessage() {

  $msg = '     ' .   $this->method . " at {$this->file}

({$this->line})\n"; if ($this->cause instanceof Exception) {

    return $msg . $this->cause->getCauseMessage();

} return $msg; } It's making my head spin a little, but that should still handle the recursion .... right?

Absolutely, seemed a little bit dumb to pass "himself" each time (fyi is now 'private'). See the latest ouput! PEAR_Exception occurred: Error message: Failed first Error code : 0 File (Line) : /home/cox/www/php5/test.php (30)

  Method       : test::first()

Nested Error :

    #test::third() at /home/cox/www/php5/test.php (45)
     Failed third
    #test::second() at /home/cox/www/php5/test.php (39)
     Failed second
    #test::first() at /home/cox/www/php5/test.php (30)
     Failed first

Cool -- very nice output. & yes private or protected makes more sense. I wonder if we want to make it optional to have full traces also as part of __toString() since Exception::__toString() does this? Maybe via static setting of the class (?) Maybe that would be overkill actually, as getTraceAsString() is pretty easy to call.

From other side, I'm thinking on constructor signatures. Nowadays we're used to just "re-throw" the same PEAR_Error, no need to attach more info to it, as would be with the current PEAR_Exception. Something like: try {

    throw new PEAR_Exception('Error Message', $code);

} catch (Exception $e) {

    throw new PEAR_Exception($e); // <--

}

Well, we could do that too. I wasn't sure if there was a lot of value in that. I know there's some overhead to adding try/catch blocks so, I tend to avoid that and let the errors bubble up. On the other hand, sometimes for consistency you want to re-throw your exceptions so that they are all of a certain class. Catch a ReflectionException and re-throw as PEAR_Exception, for example. I implemented the above signature, for example, in my PropelException, but found that in every place where I wanted to just wrap an Exception it helped to provide another message to describe the context: // e.g. simplified: try { $con->execute("DELETE FROM ....."); } catch (SQLException $sqle) { throw new PropelException("Object delete failed", $sqle); } So, it would add some flexibility, but it might be better to discourage simple catch/re-throw for performance reasons. (... on the other hand, those bubble-ups need to be made explicit in documentation since PHP doesn't have a 'throws' keyword.)

Some other minor commits are in CVS already.

Great work, Tomas. Thanks again! Hans