E_NOTICE: How useful is it REALLY to fix every one?

Question

First off I know this question has gone around more than once here:

• Why should I fix E_NOTICE errors?
• Why should I fix E_NOTICE errors? Pros and cons

But the more that I fix all E_NOTICEs (as people say you should) the more I notice that:

• I am micro-optimising
• I am actually making more code and making my code harder to mantain and slower

Take an example:

Say your using the MongoDB PHP driver and you have a MongoDate object in a class var named ts within a class that represents a single row in a collection in your database. Now you acces this var like: $obj->ts->sec but PHP throws a fit (E_NOTICE) because ts in this case is not defined as an object in itself because this particular row does not have a ts field. So you think this is OK, this is desired behaviour, if it's not set return null and I will take care of it myself outside of the interpreters own robotic workings (since you wrap this in a date() function that just returns 1970 if the var is null or a none-object).

But now to fix that E_NOTICE as another developer really wants me to since having ANY E_NOTICEs is terribad and it makes the code slower to not do it according to the errors. So I make a new function in the $obj class called getTs and I give it 3 lines, literally to do nothing but check if the ts var is a MongoDate object and return it if it is...

WHY? Can't PHP do this perfectly fine for me within its much faster interpreter than having to do it within the runtime of the app itself? I mean every where I am having to add useless bumpth to my code, pretty much empty functions to detect variables that I actually just handle with PHPs own ability to return null or checking their instanceof when I really need to (when it is vital to the operation and behaviour of the said function) and don't get me started on the isset()s I have added about 300 lines of isset()s, it's getting out of hand. I have of course got to make this getTs functions because you can't do:

class obj{
    public $ts = new MongoDate();
}

I would either have to store the ts within the __constructor (which I am not too happy about either, I am using a lot of magics as it is) or use a function to detect if it's set (which I do now).

I mean I understand why I should fix:

• Undefined vars
• Assigning properties of unset vars (null vars)
• constant errors etc

But if you have tested your code and you know it is safe and will only work the way you desire what is the point in fixing all of the undefined index or none-object errors? Isn't adding a bunch of isset()s and 2 lines functions to your code actually micro-optimisation?

I have noticed after making half my site E_NOTICE compliant that actually it uses more CPU, memory and time now...so really what's the point of dealing with every E_NOTICE error and not just the ones that ARE errors?

Thanks for your thoughts,

Answer 1

You do certainly do get better performance by using isset(). I did some benchmarks, not too long ago, and just hiding errors came out to be about 10x slower.

http://garrettbluma.com/2011/11/14/php-isset-performance/

That said, performance usually isn't a critical factor in PHP. What does, personally drive me crazy is silent errors.

When the interpreter chooses to not flag something as an error (which could lead to instability) is a huge problem. PHP in particular has a tendency to

• warn about things that should error (e.g. failure to connect to database) and
• issue notices about things that ought to warn (e.g. attempting to access a member of a null object).

Perhaps I'm just overly opinionated about this kind of stuff but I've been bitten before by these silent errors. I recommend always including E_NOTICE in error reporting.

Answer 2

Whether or not you should fix them is certainly debatable, and will just depend on the return in your situation; eg, it's more important if the code will have a longer life-span, more devs, etc.

In general, assuming that your functions will be used (and mis-used) by someone else is the best practice, so you should do isset/!empty/is_object checks to account for this. Often, your code will find it's way into uses and situations you never intended it for.

As far as performance, Every time any kind of error is thrown--E_NOTICE included--the interpreter spins up the error handler, builds a stack trace, and formats the error. The point is that, whether or not you have them reporting, errors always slow execution; therefore, 2-3 function calls to avoid an E_NOTICE will still improve your performance.

Edit: Alternatives for the above example

I wouldn't necessarily create extra objects to avoid the errors; you can gracefully avoid them without. Here are a couple of options:

1) Function that handles missing ts:

SpecialClass class {

    funciton getTs () {
        return !empty($this->ts) ? $ts->sec : false;
    }
}

2) Deal with missing ts in template/procedure:

if (!empty($obj->ts->sec)) {
    //do something
}

I particularly like empty() because you can use it to replace of (isset($var) && ($var or 0 != $var //etc)), saving multiple calls/comparisons and empty never throws notices for the target var or attribute. It will throw an error if you're calling it on a proptery/member of a non-existent variable.