Re: [PEPr] Comment on PEAR::PEAR_PackageUpdate
| From: | Scott Mattocks | Date: | Tue, 14 Mar 2006 15:47:43 +0000 |
| Subject: | Re: [PEPr] Comment on PEAR::PEAR_PackageUpdate | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-41782@lists.php.net to get a copy of this message | ||
Thanks for the comments Justin,
Justin Patrin wrote:
I would suggest using PEAR_ErrorStack instead of implementing your own error stack in the class.I am thoroughly confused by the docs for PEAR_ErrorStack but I will try to read them over again and update the package.
I'm also not sure why you're not returning a PEAR_Error. I don't see any loops in the class itself so it doesn't make sense for it to simply aggregate its errors. The package should return PEAR::raiseError on error and let the calling package decide if it wants to aggregate or not.I am not returning errors because it makes the package more difficult to use. By compiling a stack of errors, the user can write something like the example in the proposal which I think is cleaner than: $result = $ppu->checkUpdate(); if (!PEAR::isError($result)) {
$result2 = $ppu->presentUpdate();
...
}
Returning PEAR_Error every time at least doubles the amount of code needed to update a package.
Also, this package is designed as a back end for different front end packages. I think most front ends will want to collect all of the errors and then display them all at once like PEAR_PackageUpdate_Gtk2 does. I am trying to put the common functionality into the base package so that other packages don't have to implement it. If it will be more difficult for other developers to work with the stack than to handle errors on an individual basis I will change the package, but I don't see the argument for that at the moment.
As has been said before, instanceOf is PHP5 only. If this is a PHP4 package (as it seems to be) please use is_a.instanceof is only in the PEAR_PackageUpdate_Gtk2 proposal which is a PHP5 only package. PHP-GTK 2 requires at least PHP 5.1.2.
Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=369Thanks again for your comments. -- Scott Mattocks http://www.crisscott.com