Re: updated docs for Error_Raise

Alan Knowles wrote:

This looks fine for an upgrade to raiseError(). It is a BC break though.

Not really.. - it should be posible to support the old format, as arg3 used to be a int, where as the new format uses an array.. !is_array() = old formant..

Right - makes sense.

I think this approach is probably one of those personal preference ones... From what I've seen/used of gnome/gtk/php all of which currently use error strings in the text.. (gtk/gnome using gettext in g_warning etc.) - it's never really let me down (yet....) I've saw MDB yesterday, which showed justification for not using error strings (as each driver should to some degree produce the same set of drivers).. - but these factory pattern packages are less than 10% of PEAR's packages.

I think the main thing that is clearly needed is that: a) PEAR needs to require error codes, referenced by constant name, and not by number (PEAR::raiseError(6) is a disastrous idea)

YES

b) you can't require error codes to be referenced by constant name without also requiring the originating package of the error code.

YES

c) adding in the getPackage(), getErrorType() methods to PEAR_Error will enable complex error handling.

YES..

If you don't think Error_Raise has a place in PEAR_Error, I wonder if it might be bundled as a separate PFC with the changes above made to PEAR_Error. Then, PEAR_Error would be forward-compatible, and Error_Raise would be backward-compatible. As a last resort, I can see an optional default error message being provided in case error generation registration doesn't work as a 4th parameter. Error_Raise::error('package', CODE, array('param' => 'thing'), "This %param% wasn't a 'that'");

I've shown above that by using a raiseError wrapper in the main class, you dont need to keep typing 'package', each time.. - I agree its needed.. - but should it be an argument, rather than a default option on the packages raiseError wrapper...??

This all makes sense. The typo factor is large if you have to type the package name. I still think having the new error/warning/notice/exception methods is more intuitive for new users than the current raiseError or your solution, perhaps not for existing PEAR developers, who knows. There is one other concern that I hadn't realized was an assumption on my part - performance suffers slightly if we continue to use existing raiseError. My solution circumvents this problem through new methods that break BC, using existing raiseError would add a slight performance penalty. One of the most common complaints about PEAR's error handling is that the PEAR.php file adds a ton of overhead. Maybe we need to investigate a PEAR_Error_Lite error handling solution that only contains the bare minimum of requirements, and doesn't require PEAR.php. This would allow packages like Cache_Lite that really depend on load time to use advanced error handling with no penalty. I don't have concrete code to back up this idea yet. It would be a BC break in some ways, most likely. Greg