September 26, 2011

Are there any essential reasons to use isset() over @ in php

Question by Tchalvak

So I’m working on cleanup of a horrible codebase, and I’m slowly moving to full error reporting.

It’s an arduous process, with hundreds of notices along the lines of:

Notice: Undefined index: incoming in /path/to/code/somescript.php on line 18

due to uses of variables assuming undefined variables will just process as false, like:

if($_SESSION['incoming']){
    // do something
}

The goal is to be able to know when a incorrectly undefined variable introduced, the ability to use strict error/notice checking, as the first stage in a refactoring process that -will- eventually include rewriting of the spots of code that rely on standard input arrays in this way. There are two ways that I know of to replace a variable that may or may not be defined
in a way that suppresses notices if it isn’t yet defined.

It is rather clean to just replace instances of a variable like $_REQUEST['incoming'] that are only looking for truthy values with

@$_REQUEST['incoming'].

It is quite dirty to replace instances of a variable like $_REQUEST['incoming'] with the “standard” test, which is

(isset($_REQUEST['incoming'])? $_REQUEST['incoming'] : null)

And you’re adding a ternary/inline if, which is problematic because you can actually nest parens differently in complex code and totaly change the behavior.

So…. …is there any unacceptable aspect to use of the @ error suppression symbol compared to using (isset($something)? $something : null) ?

Edit: To be as clear as possible, I’m not comparing “rewriting the code to be good” to “@”, that’s a stage later in this process due to the added complexity of real refactoring. I’m only comparing the two ways (there may be others) that I know of to replace $undefined_variable with a non-notice-throwing version, for now.

Answer by user187291

Another option, which seems to work well with lame code that uses “superglobals” all over the place, is to wrap the globals in dedicated array objects, with more or less sensible [] behaviour:

class _myArray implements ArrayAccess, Countable, IteratorAggregate
{
     function __construct($a) {
       $this->a = $a;
     }

    // do your SPL homework here: offsetExists, offsetSet etc

    function offsetGet($k) { 
        return isset($this->a[$k]) ? $this->a[$k] : null;
        // and maybe log it or whatever
    }
}

and then

 $_REQUEST = new _myArray($_REQUEST);

This way you get back control over “$REQUEST” and friends, and can watch how the rest of code uses them.

Answer by Starx

You answered you question yourself. It suppress error, does not debug it.

...

Please fill the form - I will response as fast as I can!