Why Suppressing Notices is Wrong

When I first came to work for my current employer and opened up Zend Studio (don’t worry, they’ve mostly abandoned that atrocity of an IDE), around 2,000 of the 10,000 PHP files that made up our application lit up with errors. The developers before me never properly maintained it, and the horrors of legacy code I was exposed to were unfathomable. I knew right then and there that this wasn’t going to be an easy trip.

After spending weeks trying (and failing) to fix the mess, I gave up and focused on the more contemporary tasks implementing the new requested features on top of the chaos – a bad idea as most will agree. A job is a job though, and for a while it all worked well. In time, we started to notice the app’s write operations peaking beyond normal limits. Opening the error log revealed why: notices and warnings.

The PHP notice suppression operator is somewhat of a controversial topic in many circles. Some overuse it, some don’t use it at all, and some don’t even know it exists. Apologies in advance for the horrible code you’re about to witness in this article, but it serves a purpose to illustrate the usefulness (or lack thereof) of the suppression operator.

Notices and Warnings

PHP has different error levels categorized by severity. Notices and warnings are the weakest type which means they don’t break the execution of your application – instead, they’re just echoed out to the screen or written to the error log (or both, depending on your settings), and execution continues.

An example would be the following. If you have an empty array $aArray, and try to access a key that doesn’t exist like so:

<?php
$aArray = array();
$aArray['someKey'];

You get a notice that the key doesn’t exist.

Notice: Undefined index: someKey in /var/www/index.php on line 3

Call Stack:
    0.0009     328920   1. {main}() /var/www/index.php:0 

You get a notice that the key doesn’t exist. But this doesn’t break your code – in fact, if you try storing $aArray['someKey'] into a variable, that variable will actually get initialized as expected with a null value.

Let’s take a look at a more concrete example. Assume we have a controller that passes into its view an array of messages; the array can look like this (and in a lot of cases in this legacy code I was introduced to, it did!):

<?php
// $oView = ... our view object
// $aMessages = array();

if (/* some expression that signifies an error */) {
    $aMessages['error'] = 'Error no.2 occurred, contact developer!'; 
}

$aMessages['status'] = 'Finished operation';
$oView->aMessages = $aMessages;

You’ll notice that the “error” key is only set if the error actually occurs, that is, if the if-expression in the parentheses evaluates true.

The view code looked something like this (simplified for the purposes of this article):

<h1>Operation completed</h1>
<?php 
    echo @$this->aMessages['error'];
    echo '<hr />';
    echo $this->aMessages['status'];
    echo '<hr />';
    // ...

The status of the operation and the error is echoed out. But seeing as they were aware of the fact that “error” might not be set, they used the suppression operator @ to prevent the notice from being printed on screen.

A surprising amount of PHP developers don’t actually know that even if you suppress a notice on screen that it still gets logged if logging is turned on – so your error log is getting littered with pointless and relatively large lines of text. Developers often think @ is just legit shorthand for “ignore”, but it’s not – it’s shorthand for “I know my code here sucks, but let’s pretend everything is OK because I’m not going to make it better.”

When to Use It?

So when is using suppressions actually acceptable?

Never!

When you use suppressions, it means you’re aware your code might suck but you just don’t care. This tells developers who run into your code after you that you’re a bad developer.

It also slows down your app because it constantly has to write something into the error log. Imagine the code above being called by 1,000,000 visitors on a high traffic day. That’s one million lines of text in the error log! The impact on performance is hardly negligible.

Really? Never?

While developing, the error messages are there for a reason. They help you find bugs in your code before you inflict the same fate on your users and customers. They point you in the right direction as to what’s wrong and what needs to be fixed. This is true for notices as much as it is for fatal errors – they all serve to help you, the developer, make the best product possible. Sometimes, however, you might need to test some functionality before handling any remaining notices that are left over from before – when outputting generated files for example. If a notice gets printed before the file generation begins, that means headers were already sent (because without headers your browser has no idea how to echo the content it is being given) and that means your file won’t display properly because it needs to send its own headers to the browser in order for the browser to recognize we’re dealing with an actual file.

There’s also the rare case of having to deal with some of PHP’s iffy implementation details – for example, using the mail() function will throw an error if the mail server isn’t up, so doing @mail() is justified.

But these are all exceptions to the general rule. With training, you’ll be able to recognize the instances in which using a suppressor is unavoidable. Then and only then should you suppress anything, but you should strive to avoid even this. A good developer fixes ALL possible notices as soon as they detect a chance of encountering them.

How can you force yourself to do this, you might ask? By doing the same thing I did when I built my framework:

<?php
function myErrorHandler($errno, $errstr, $errfile, $errline)
{
    if (!(error_reporting() & $errno)) {
        // This error code is not included in error_reporting         
        return;
    }
    echo "<b>ERROR!</b> [$errno] $errstr<br>n";
    echo "  Fatal error on line $errline in file $errfile";
    echo ", PHP " . PHP_VERSION . " (" . PHP_OS . ")<br>n";
    echo "Aborting...<br>n";
    exit(1);
    break;
}
$old_error_handler = set_error_handler("myErrorHandler");

The snippet above replaces the default error handler with a custom function, which makes the application die on absolutely any error, warning, or notice.

This is, of course, simplified. A proper error reporting class is far more advanced than this and outputs much more info, but the gist of it is as follows: break your app on purpose whenever a notice or warning is encountered, or, in other words, treat your warnings and notices as fatal errors.

By doing this, your app will break during development and testing, giving you the chance to write your code as bulletproof as possible. You’re making your life and the lives of those coming into your code after you’re gone all much easier.

Adding a custom error handler which breaks your app with every notice and warning will make sure you and your developers take much more care of the cleanliness of your code. In the long term, it keeps the error log clean and reserved for only the most serious of errors. It might slow down development a bit for rookie programmers, but experienced developers will appreciate this extra effort. After all, it’s better to spend an extra week writing cleaner code than to spend months later down the line trying to fix it.

Conclusion

Good code is a medal you can strap onto your chest. It’s a trophy you wear proudly. Good code follows you after you leave it behind, and the tales it tells to those that inherit it will feature you as the main character. It’s up to you to make sure you’re the protagonist of those stories, and not an antagonist.