Re: updated docs for Error_Raise

Alan Knowles wrote:

which would mean a full on version looking like.. - PEAR:raiseError(

    "file %s was not found in path %s",  // beginners level..
    ERR_NO, // reasonable quality essentail..
    array(  //options - if 3rd arg is array .. assume new format.
        'args'=>  array($file,$path),
        'package'=>'MyPackage',
        'callback'=>array($object,'method'),
        'trigger' => E_WARN,
        'actionType' => PEAR_ERROR_DIE,
        'errorType' => 'warning'
            // default applied by error::warning()
           )

);

This looks fine for an upgrade to raiseError(). It is a BC break though. The solution I'm proposing actually would maintain 100% BC with PEAR_Error as raiseError() would allow the setting of a default error message - note that if there IS a default error message (it is not ''), the Error_Raise_Error->getMessage() method simply returns parent::getMessage(). So, if someone simply uses PEAR::raiseError("error message"), then it would still work - error message returned as normal. I have a serious problem with PEAR_Error that has little to do with your example, but affects it. PEAR_Error mixes error raising and error handling. This is not a good idea. Whether a callback should be used, an error triggered, or thrown is a feature of error handling that should be hidden from the package that raises an error. The only responsibility a package should have is to indicate that there is an error condition, and let other code handle the error. With this amendment, I'd like to see only 'args', 'package', and 'errorType' in the options array, and require other methods like setErrorHandling() to take care of the error handling features. Allowing error handling insertion in raiseError() was a mistake What I really can't understand is why this should be forced on new code - this is why I chose new method names. Instead of overriding raiseError(), there are the new methods warning, error, notice, exception, and these methods are designed for use by those who wish to use advanced error handling. I don't think the core should force people to do anything other than provide an error code and a package name. Tying package to errors is *absolutely* essential - here's why: // package a <?php define('A_ERROR_BLAH', 1); ?> // package b uses package a <?php define('B_ERROR_SOMETHING', 1); ?> // application c uses b <?php if (PEAR::isError($e)) { switch ($e->getCode()) {

      case A_ERROR_BLAH :
         // uhoh - A_ERROR_BLAH == B_ERROR_SOMETHING, how do we know which package threw the error?
      break;

} } ?> In other words, every package would have to have a subset of integers assigned to them - what if the codes were not enough, or a typo causes an error to drift into another package's error codes? This is REAL overhead :). Requiring a package name is simple, efficient, and much more natural.

This makes a complete disassociation between error definition and message.. - when you are writing the real message you have to cross reference the error location... to work out what the error was supposed to say.. - It's a penalty I am beginning to find in a number of places where I've abstracted stuff to far recently.. (look up 3 files to find what was variables/etc. to use..)

I know this is starting to feel like a tennis match, but I do have a counter to your reply :) It seems to me that this couldn't happen - you find a situation that needs to return a new error. First, you go to the single place that you define your error codes, and define a new code, then you add an error message to the callback handler, then you return to programming. Later on, another similar situation pops up. You go look at the single place that defined error codes, and read the code and its documentation (what error condition does this code cover?) If error messages and error codes *aren't* defined in the same location, how do you find them at all? Error_Raise encourages developers to define all of their error constants and place their error callback right next to one another. Using error messages in the source scattered about seems to be what you're suggesting, perhaps I misinterpreted? This would seem to introduce several critical problems: -the same error message must be typed and maintained in several locations that return the same error - typos are a serious issue then. -if you only know the code, and want to change the message returned, you can't look at the place where a code was defined, you have to find the place that uses the code in the source - and change every single occurence. With an error message generator, there is 1->1 code to message, allowing both an easier time of finding the error code and error message. 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) b) you can't require error codes to be referenced by constant name without also requiring the originating package of the error code. c) adding in the getPackage(), getErrorType() methods to PEAR_Error will enable complex error handling. 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'"); Would this satisfy your needs? Greg