4

I've noticed on XAMPP that strict error reporting is on and I get undefined index errors now. I just have two small questions (I'm still learning here):

I know you don't have to declare variables in PHP but is there any advantage to declaring them anyway? If not, why do I get errors when strict error reporting is on when I don't define them?

When I use get variables for example, I check for their value before I run a function like

if($_GET['todo'] == 'adduser')
    runFunctionAddUser();

This gives an error because I never check if the get variable exists first. Should I do

if(isset($_GET['todo']))
    if($_GET['todo'] == 'adduser')
        runFunctionAddUser();

instead? Would there be an advantage to this or would it be unnecessary and slow?

tekknolagi
  • 10,663
  • 24
  • 75
  • 119
David Zorychta
  • 13,039
  • 6
  • 45
  • 81
  • May I recommend [The Definitive Guide To PHP's isset And empty](http://kunststube.net/isset/). – deceze Dec 07 '11 at 01:36
  • The buffer overflow thing is a myth. -- My personal opinion (and I'm being pretty alone with that) is that suppressing notices should be avoided, unless they get too annoying and plentiful. Absent $_GET variables in particular make useful debug messages to audit the application flow. Keep them, but disable error display on your production server. Keep them occasionally enabled for development and debugging though. – mario Dec 07 '11 at 01:37

4 Answers4

14

This thread is abit old but I did some tests relating to the question so I might as well post it:

The testing code (PHP 5.3.3 - CentOS release 6.5 (Final)):

class NonExistant
{
    protected $associativeArray = array(
        'one' => 'one',
        'two' => 'two',
        'three' => 'three',
        'four' => 'four',
        'five' => 'five',
        'six' => 'six',
    );

    protected $numIterations = 10000;

    public function noCheckingTest()
    {
        for ($i = 0; $i < $this->numIterations; $i++) {
            $this->associativeArray['none'];
        }
    }

    public function emptyTest()
    {
        for ($i = 0; $i < $this->numIterations; $i++) {
            empty($this->associativeArray['none']);
        }
    }

    public function isnullTest()
    {
        for ($i = 0; $i < $this->numIterations; $i++) {
            is_null($this->associativeArray['none']);
        }
    }


    public function issetTest()
    {
        for ($i = 0; $i < $this->numIterations; $i++) {
            isset($this->associativeArray['none']);
        }
    }

    public function arrayKeyExistsTest()
    {
        for ($i = 0; $i < $this->numIterations; $i++) {
            array_key_exists('none', $this->associativeArray);
        }
    }
}

The results are:

| Method Name                              | Run time             | Difference
=========================================================================================
| NonExistant::noCheckingTest()            | 0.86004090309143     | +18491.315775911%
| NonExistant::emptyTest()                 | 0.0046701431274414   | +0.95346080503016%
| NonExistant::isnullTest()                | 0.88424181938171     | +19014.461681183%
| NonExistant::issetTest()                 | 0.0046260356903076   | Fastest
| NonExistant::arrayKeyExistsTest()        | 1.9001779556274      | +209.73055713%

All the functions are called the same way with via call_user_func() and timed with microtime(true)

Observations

empty() and isset() are the clear winners over the other 2 methods here, the two methods are pretty much tied on performance.

is_null() performs bad because it needs to lookup the value first, pretty much the same as just accessing the non-existant value $this->associativeArray['none'], which involves a full lookup of the array.

However, i'm surprised by the performance of array_key_exists(). Where it is 2 times slower than empty() and isset().

NOTE:

All the functions I've tested have different uses, this benchmark is only here for the most generic use case where you want to quickly check the value in an array. We can go into argument of whether null should be considered a "value" or simply an indicator of non-existence, but that's another topic. o.o

UPDATE 2017-01-20

Use PHP 7.1

Fixed bug mentioned by @bstoney

$ php -v
PHP 7.1.0 (cli) (built: Dec  2 2016 03:30:24) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.1.0-dev, Copyright (c) 1998-2016 Zend Technologies
$ php -a
php > $a = ['one' => 1, 'two' => 2, 'three' => 3];
php > $numIterations = 1000000;
php > $start = microtime(true); for ($i = 0; $i < $numIterations; $i++) { $a['none']; }; echo microtime(true) - $start;
0.43768811225891
php > $start = microtime(true); for ($i = 0; $i < $numIterations; $i++) { empty($a['none']); }; echo microtime(true) - $start;
0.033049821853638
php > $start = microtime(true); for ($i = 0; $i < $numIterations; $i++) { is_null($a['none']); }; echo microtime(true) - $start;
0.43995404243469
php > $start = microtime(true); for ($i = 0; $i < $numIterations; $i++) { isset($a['none']); }; echo microtime(true) - $start;
0.027907848358154
php > $start = microtime(true); for ($i = 0; $i < $numIterations; $i++) { array_key_exists('none', $a); }; echo microtime(true) - $start;
0.049405097961426
Eric Lindsey
  • 954
  • 8
  • 19
Populus
  • 7,470
  • 3
  • 38
  • 54
  • 1
    The test code using `array_key_exists` is not valid and should be `array_key_exists('none', $this->associativeArray)` – bstoney Jan 20 '17 at 07:20
  • 1
    @bstoney good catch, only took 3 years for someone to notice XD – Populus Jan 20 '17 at 22:12
  • Updated answer to include PHP 7.1 tests and fixed bug :) – Populus Jan 20 '17 at 22:33
  • Thanks. I found another possibility too: using `$value = &$this->associativeArray['none'];` you can then check for `$value === null`. The performance of this is only slightly slower than `isset` – bstoney Jan 22 '17 at 23:56
  • Also worth noting that `array_key_exists` is the only one of these options that can determine if an array item has not been set or that it has been set to null – bstoney Jan 23 '17 at 00:04
  • TBH we should almost always use `array_key_exists` for key existence checking; however, sometimes bad code results in arrays with keys set to null, when it's supposed to be null, sometimes this is done on libraries you depend on, so doing `isset` or `empty` check is good enough, you really shouldn't have keys set to null anyway unless null means something explicitly, which is why you see so much code out there relying on `isset`. As for your idea with the whole reference thing, I suggest you don't do that, you might as well do `$array['none'] !== null`, references in PHP are basically aliases. – Populus Jan 27 '17 at 07:19
  • The performance of `$value = &$array['none']; $value !== null;` is completely different to `$array['none'] !== null;`. Try it. – bstoney Jan 30 '17 at 03:31
  • 2
    Instead of is_null, you should have done `===null`. It's likely faster. – Pacerier Oct 02 '17 at 02:19
1

I would suggest if(!empty($_GET['todo']) && $_GET['todo'] == 'adduser'). This works because PHP short-circuits logic expressions.

It's best to develop using an error reporting level of E_ALL (which includes E_NOTICE) because it quickly helps identify mistyped variable / key names.

If you're asking whether or not testing for the existence of a variable is slow, you're asking the wrong question (the answer is that it's blazing fast. Your slowness comes in working with the filesystem / database / etc).

Kenaniah
  • 5,171
  • 24
  • 27
  • Thanks for replying. I'll make sure to do empty checks from now on, but do you know why it's better to check with empty() and isset() in the first place instead of just not doing it? – David Zorychta Dec 07 '11 at 01:24
  • It lets the language know that you were expecting that the variable *may not* exist. Take for example `$_GET['foobr'] = 'blah'; if($_GET['foobar']) ...;`. Because a notice is thrown, the misspelling is easily identified. – Kenaniah Dec 07 '11 at 01:26
  • Think of it like a semi-automatic testing suite that ensures your usage of variables is (somewhat) sane. – Kenaniah Dec 07 '11 at 01:28
  • **Don't** use `!empty` when you mean `isset`. Suppose the value is something that equals empty - `0`, `false`, an empty string... – ToolmakerSteve Apr 25 '19 at 17:10
0

It's not an error about a missing variable you're getting, it's an error about trying to compare variables to a non-existent variable.

If you try to compare nothingness to 'adduser' it throws an error, because you can't compare nothingness.

That's the answer to #1.

For #2, you SHOULD almost always (if not always) check for the existence of your variables, especially your GET variables, because they're so volatile.

rockerest
  • 10,412
  • 3
  • 37
  • 67
  • Thanks for helping to clear that up, for all the variables I use then, should I do $variable = ''; for each time I'm doing something where I later use the variable? – David Zorychta Dec 07 '11 at 01:37
0

Thats not a direct answer because you already got that, but an information how to archive it:

I always use a function like this in all my programs to initialize the incoming variables (with a default value if needed).

So you could do your initializing like this:

$todo = initGet('todo');

And then just check normally:

if($todo=='adduser') {

And the function:

function initGet($var, $default=''){
    if(!isset($_GET[$var])) return $default;
    return $_GET[$var];
}
Community
  • 1
  • 1
Marc
  • 6,749
  • 9
  • 47
  • 78