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

Question

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 1

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 2

You need to decide on your own if you rate the @ usage acceptable or not. This is hard to rate from a third party, as one needs to know the code for that.

However, it already looks like that you don't want any error suppression to have the code more accessible for you as the programmer who needs to work with it.

You can create a specification of it in the re-factoring of the code-base you're referring to and then apply it to the code-base.

It's your decision, use the language as a tool.

You can disable the error suppression operator as well by using an own callback function for errors and warnings or by using the scream extension or via xdebug's xdebug.scream setting.

Answer 3

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

Answer 4

In my opinion you should be using the isset() method to check your variables properly.

Suppressing the error does not make it go away, it just stops it from being displayed because it essentially says "set error_reporting(0) for this line", and if I remember correctly it would be slower than checking isset() too.

And if you don't like the ternary operator then you should use the full if else statement. It might make your code longer but it is more readable.

Answer 5

I would never suppress errors on a development server, but I would naturally suppress errors on a live server. If you're developing on a live server, well, you shouldn't. That means to me that the @ symbol is always unacceptable. There is no reason to suppress an error in development. You should see all errors including notices.

@ also slows things down a bit, but I'm not sure if isset() is faster or slower.

If it is a pain to you to write isset() so many times, I'd just write a function like

function request($arg, $default = null) {
   return isset($_REQUEST[$arg]) ? trim($_REQUEST[$arg]) : $default;
}

And just use request('var') instead.

Answer 6

I've actually discovered another caveat of the @ beyond the ones mentioned here that I'll have to consider, which is that when dealing with functions, or object method calls, the @ could prevent an error even through the error kills the script, as per here:

http://us3.php.net/manual/en/language.operators.errorcontrol.php

Which is a pretty powerful argument of a thing to avoid in the rare situation where an attempt to suppress a variable notice suppressed a function undefined error instead (and perhaps that potential to spill over into more serious errors is another unvoiced reason that people dislike @?).

Answer 7

Most so-called "PHP programmers" do not understand the whole idea of assigning variables at all.
Just because of lack of any programming education or background.

Well, it isn't going a big deal with usual php script, coded with considerable efforts and consists of some HTML/Mysql spaghetti and very few variables.

Another matter is somewhat bigger code, when writing going to be relatively easy but debugging turns up a nightmare. And you are learn to value EVERY bloody error message as you come to understanding that error messages are your FRIENDS, not some irritating and disturbing things, which better to be gagged off.

So, upon this understanding you're learn to leave no intentional errors in your code.
And define all your variables as well.
And thus make error messages your friends, telling you that something gone wrong, lelping to hunt down some hard-spotting error which caused by uninitialized variable.

Another funny consequence of lack of education is that 9 out of 10 "PHP programmers" cannot distinguish error suppression from turning displaying errors off and use former in place of latter.