Re: last Pear.php.in and DB.php commit
| From: | Tomas V.V.Cox | Date: | Tue, 17 Apr 2001 15:52:03 +0000 |
| Subject: | Re: last Pear.php.in and DB.php commit | ||
| References: | 1 2 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-248@lists.php.net to get a copy of this message | ||
Stig Sæther Bakken wrote:
>
> I have one issue with your fix though...
>
> > @@ -190,7 +190,7 @@
> >
> > function setErrorHandling($mode, $options = null)
> > {
> > - if (isset($this)) {
> > + /*if (isset($this)) {
> > $setmode = &$this->_default_error_mode;
> > $setoptions = &$this->_default_error_options;
> > $setcallback = &$this->_default_error_callback;
> > @@ -198,7 +198,9 @@
> > $setmode = &$GLOBALS['_PEAR_default_error_mode'];
> > $setoptions = &$GLOBALS['_PEAR_default_error_options'];
> > $setcallback = &$GLOBALS['_PEAR_default_error_callback'];
> > - }
> > + }*/
> > + $GLOBALS['_PEAR_default_error_mode'] = $mode;
> > + $GLOBALS['_PEAR_default_error_options'] = $options;
>
> Here you lose the distinction between:
>
> PEAR::setErrorHandling(PEAR_ERROR_DIE);
>
> and
>
> $obj->setErrorHandling(PEAR_ERROR_DIE);
>
> (The former variant sets the default error handling for _all_ objects,
> while the latter one sets it for one object.) Or did I misunderstand
> your fix?
>
> Here's a suggestion: modify a test (or make a new one) that
> demonstrates how the code is broken, and what the test results should
> be with correct behaviour. Communicating with code is good, and
> communicating with tests is better. :-)
>
> If you just send me an example, I can make the test.
<?php
function handle_error ($obj) {
die ($obj->getMessage());
}
PEAR::setErrorHandling(PEAR_ERROR_CALLBACK, 'handle_error');
$dsn = 'pgsql://postgres@localhostNONONO/no_db';
$conn = DB::connect($dsn);
echo "no error";
?>
The output of this is "no error". This is because, DB_extensions don't
use PEAR::raiseError (I think in all Pear clases errors should be raised
with this) and, because they have a default error_mode they'll never
notice that there are a global error mode (this is why I added the
GLOBAL stuff to PEAR_Error constructor, let's say a temporary fix). Also
I see that PEAR::setErrorHandling() do nothing (it simply set some
private vars?).
In your comment about PEAR::setErrorHandling vs $obj->setErrorHandling,
you are correct. I thinked that if DB, has its own DB::setErrorHandling
is no need for it, but I'm wrong. I think the correct
PEAR.php->setErrorHandling is:
function setErrorHandling($mode, $options = null)
{
switch ($mode) {
.... // checks (sorry coding in Netscape is hard ;)
}
// something like if passed checks
if (isset($this)) {
$this->_default_error_mode = $setmode;
$this->_default_error_options = $setoptions;
$this->_default_error_callback = $setcallback;
} else {
$GLOBALS['_PEAR_default_error_mode'] = $setmode;
$GLOBALS['_PEAR_default_error_options'] = $setoptions;
$GLOBALS['_PEAR_default_error_callback'] = $setcallback;
}
}
And change the default error_mode property in common.php to
_default_error_mode (and of course the others).
Other minor stuff: changed the PEAR_ERROR_CALLBACK checks with some IMHO
better checks like "function_exists" or "method_exists".
Tomas V.V.Cox