Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I'm having a debate with a colleague over the correct usage (if any) of trigger_error in the context of magic methods. Firstly, I think that trigger_error should be avoided except for this one case.

Say we have a class with one method foo()

class A {
    public function foo() {
        echo 'bar';
    }
}

Now say we want to provide the exact same interface but use a magic method to catch all method calls

class B {
    public function __call($method, $args) {
        switch (strtolower($method)) {
        case 'foo':
            echo 'bar';
            break;
        }
    }
}

$a = new A;
$b = new B;

$a->foo(); //bar
$b->foo(); //bar

Both classes are the same in the way they respond to foo() but differ when calling an invalid method.

$a->doesntexist(); //Error
$b->doesntexist(); //Does nothing

My argument is that magic methods should call trigger_error when a unknown method is caught

class B {
    public function __call($method, $args) {
        switch (strtolower($method)) {
        case 'foo':
            echo 'bar';
            break;
        default:
            $class = get_class($this);
            $trace = debug_backtrace();
            $file = $trace[0]['file'];
            $line = $trace[0]['line'];
            trigger_error("Call to undefined method $class::$method() in $file on line $line", E_USER_ERROR);
            break;
        }
    }
}

So that both classes behave (almost) identically

$a->badMethod(); //Call to undefined method A::badMethod() in [..] on line 28
$b->badMethod(); //Call to undefined method B::badMethod() in [..] on line 32

My use-case is an ActiveRecord implementation. I use __call to catch and handle methods that essentially do the same thing but have modifiers such as Distinct or Ignore, e.g.

selectDistinct()
selectDistinctColumn($column, ..)
selectAll()
selectOne()
select()

or

insert()
replace()
insertIgnore()
replaceIgnore()

Methods like where(), from(), groupBy(), etc. are hard-coded.

My argument is highlighted when you accidentally call insret(). If my active record implementation hardcoded all of the methods then it would be an error.

As with any good abstraction, the user should be unaware of the implementation details and rely solely on the interface. Why should the implementation that uses magic methods behave any differently? Both should be an error.

share|improve this question

4 Answers 4

IMO, this is a perfectly valid use case for trigger_error :

function handleError($errno, $errstring, $errfile, $errline, $errcontext) {
    if (error_reporting() & $errno) {
        // only process when included in error_reporting
        return handleException(new \Exception($errstring, $errno));
    }
    return true;
}

function handleException($exception){
    // Here, you do whatever you want with the generated
    // exceptions. You can store them in a file or database,
    // output them in a debug section of your page or do
    // pretty much anything else with it, as if it's a
    // normal variable

    switch ($code) {
        case E_ERROR:
        case E_CORE_ERROR:
        case E_USER_ERROR:
            // Make sure script exits here
            exit(1);
        default:
            // Let script continue
            return true;
    }
}

// Set error handler to your custom handler
set_error_handler('handleError');
// Set exception handler to your custom handler
set_exception_handler('handleException');


// ---------------------------------- //

// Generate warning
trigger_error('This went wrong, but we can continue', E_USER_WARNING);

// Generate fatal error :
trigger_error('This went horrible wrong', E_USER_ERROR);

Using this strategy, you get the $errcontext parameter if you do $exception->getTrace() within the function handleException. This is very useful for certain debugging purposes.

Unfortunately, this works only if you use trigger_error directly from your context, which means you can't use a wrapper function/method to alias the trigger_error function (so you can't do something like function debug($code, $message) { return trigger_error($message, $code); } if you want the context data in your trace).

I have been looking for a better alternative, but so far I haven't found any.

share|improve this answer

Standard PHP errors should be considered obsolete. PHP provides a built-in class ErrorException for converting errors, warnings, and notices into exceptions with a full proper stack trace. You use it like this:

function errorToExceptionHandler($errNo, $errStr, $errFile, $errLine, $errContext)
{
if (error_reporting() == 0) return;
throw new ErrorException($errStr, 0, $errNo, $errFile, $errLine);
}
set_error_handler('errorToExceptionHandler');

Using that, this question becomes moot. Built-in errors now raise exceptions and so your own code should as well.

share|improve this answer
1  
If you use this, you need to check the type of the error, lest you turn E_NOTICEs into exceptions. That would be bad. –  Matthew Scharley Apr 1 '11 at 7:37
1  
No, turning E_NOTICES into Exceptions is good! All notices should be considered errors; that's good practice whether you're converting them to exceptions or not. –  Wayne Apr 1 '11 at 16:51
1  
Normally I'd agree with you, but the second you start using any third party code, this quickly tends to fall on its face. –  Matthew Scharley Apr 1 '11 at 21:17
    
Almost all 3rd party libraries are E_STRICT | E_ALL safe. If I'm using code that isn't, I'll go in and fix it. I've worked this way for years without issue. –  Wayne Apr 1 '11 at 23:16
    
you've obviously not used Dual before. The core is really good like this but many, many third party modules aren't –  Matthew Scharley Apr 5 '11 at 6:36

Take two implementations of the same ActiveRecord interface (select(), where(), etc.)

class ActiveRecord1 {
    //Hardcodes all methods
}

class ActiveRecord2 {
    //Uses __call to handle some methods, hardcodes the rest
}

If you call an invalid method on the first class, e.g. ActiveRecord1::insret(), the default PHP behaviour is to trigger an error. An invalid function/method call is not a condition that a reasonable application would want to catch and handle. Sure, you can catch it in languages like Ruby or Python where an error is an exception, but others (JavaScript / any static language / more?) will fail.

Back to PHP - if both classes implement the same interface, why shouldn't they exhibit the same behaviour?

If __call or __callStatic detect an invalid method, they should trigger an error to mimic the default behaviour of the language

$class = get_class($this);
$trace = debug_backtrace();
$file = $trace[0]['file'];
$line = $trace[0]['line'];
trigger_error("Call to undefined method $class::$method() in $file on line $line", E_USER_ERROR);

I'm not arguing whether errors should be used over exceptions (they 100% shouldn't), however I believe that PHP's magic methods are an exception - pun intended :) - to this rule in the context of the language

share|improve this answer
1  
Sorry, but I don't buy it. Why should we be sheep because exceptions didn't exist in PHP over a decade ago when classes were first implemented in PHP 4.something? –  Matthew Scharley Apr 1 '11 at 2:07
    
@Matthew for consistency. I'm not debating exceptions vs. errors (there is no debate) or whether PHP has design fails (it does), I'm arguing that in this very unique circumstance, for consistency, it's best to mimic the behaviour of the language –  chriso Apr 1 '11 at 2:13
    
Consistency isn't always a good thing. A consistent but poor design is still a poor design that's a pain to use at the end of day. –  Matthew Scharley Apr 1 '11 at 2:16
    
@Matthew true, but IMO triggering an error on an invalid method call isn't poor design 1) many (most?) languages have it built-in, and 2) I can't think of a single case where you would ever want to catch an invalid method call and handle it? –  chriso Apr 1 '11 at 2:20
    
@chriso: The universe is sufficiently large that there are use cases that neither you or I would ever dream of happening that happen. You are using __call() to do dynamic routing, is it really so unreasonable to expect that somewhere down the track someone may want to handle the case where that fails? At any rate, this is going in circles, so this will be my last comment. Do what you will, at the end of the day this comes down to a judgement call: Better support vs consistency. Both methods will result in the same effect on the application in the case of no special handling. –  Matthew Scharley Apr 1 '11 at 2:33

I'm going to throw my opinionated opinion out there, but if you use trigger_error anywhere, then you are doing something wrong. Exceptions are the way to go.

Advantages of exceptions:

  • They can be caught. This is a huge advantage, and should be the only one you need. People can actually try something different if they expect there's a chance something may go wrong. Error's don't give you this opportunity. Even setting up a custom error handler doesn't hold a candle to simply catching an exception. Regarding your comments in your question, what a 'reasonable' application is depends entirely on the context of the application. People can not catch exceptions if they think it'll never happen in their case. Not giving people a choice is a Bad Thing™.
  • Stack traces. If something goes wrong, you know where and in what context the problem happened. Have you ever tried to track down where an error is coming from some of the core methods? If you call a function with too few parameters you get a useless error which highlights the start of the method you are calling, and totally leaves out where it is calling from.
  • Clarity. Combine the above two and you get clearer code. If you try to use a custom error handler to handle errors (eg. to generate stack traces for errors), all your error handling is in one function, not where the errors are actually being generated.

Addressing your concerns, calling a method that doesn't exist may be a valid possibility. This depends entirely on the context of the code you are writing, but there are some cases where this may happen. Addressing your exact use-case, some database servers may allow some functionality that others don't. Using try/catch and exceptions in __call() vs. a function to check capabilities is a different argument altogether.

The only use-case I can think of for using trigger_error is for E_USER_WARNING or lower. Triggering an E_USER_ERROR though is always an error in my opinion.

share|improve this answer
    
Thanks for the response :) - 1) I agree that exceptions should almost always be used over errors - all of your arguments are valid points. However.. I think that in this context your argument fails.. –  chriso Apr 1 '11 at 1:32
    
"calling a method that doesn't exist may be a valid possibility" - I completely disagree. In every language (that I've ever used), calling a function/method that doesn't exist will result in an error. It is not a condition that should be caught and handled. Static languages won't let you compile with an invalid method call and dynamic languages will fail once they reach the call. –  chriso Apr 1 '11 at 1:33
    
If you read my second edit you'll see my use-case. Say you have two implementations of the same class, one using __call and one with hardcoded methods. Ignoring the implementation details, why should both classes behave differently when they implement the same interface? PHP will trigger an error if you call an invalid method with the class that has hardcoded methods. Using trigger_error in the context of __call or __callStatic mimics the default behaviour of the language –  chriso Apr 1 '11 at 1:35
    
@chriso: Dynamic languages that aren't PHP will fail with an exception that can be caught. Ruby for instance throws a NoMethodError which you can catch if you so desire. Error's are a giant mistake in PHP in my personal opinion. Just because the core uses a broken method for reporting errors doesn't mean your own code should. –  Matthew Scharley Apr 1 '11 at 1:41
    
@chriso I like to believe the only reason that core still uses errors is for backwards compatibility. Hopefully PHP6 will be another leap forward like PHP5 was and drop errors altogether. At the end of the day, both errors and exceptions generate the same result: an immediate termination of executing code. With an exception you can diagnose why and where so much easier though. –  Matthew Scharley Apr 1 '11 at 1:44

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.