Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

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.

like image 725
Kzqai Avatar asked Dec 11 '25 16:12

Kzqai


2 Answers

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.

like image 124
user187291 Avatar answered Dec 14 '25 05:12

user187291


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.

like image 20
hakre Avatar answered Dec 14 '25 06:12

hakre