Apr 142009
 

I was working on one of my php free software projects this weekend, and found an interesting bug on one of my pages, resulting in a massive number of database lookups (203 for one page) where ~180 of them was the same “kind” of lookup. Investigating further, I found out that one of my functions which was using isset() was returning a different result than what I would’ve expected.

PHP isset()  has (to me) a very strange behaviour – not only does it check to see if your variable is set (defined), it also wants it to have a specific value. Or, not so much a specific value, but it doesn’t want it to be null. Which means that running:

echo isset($variable) ? ‘yes’ : ‘no’;
$variable = null;
echo isset($variable) ? ‘yes’ : ‘no’;
$variable = ‘anything’;
echo isset($variable) ? ‘yes’ : ‘no’;

will return “no” in the two first cases, while “yes” only on the third attempt.

This is apparently intended behaviour, according to php.net/isset:

Determine whether a variable is set.
If a variable has been unset with unset(), it will no longer be set. isset() will return FALSE if testing a variable that has been set to NULL.

In my case, I was maintaining a $settings array with keys that matched the setting. Instead of loading all settings at startup, I would load the general settings, and only load the user-specific settings on demand. This meant that I would populate the array with keys for the settings that were loaded, as they were loaded. If the key didn’t exist, it would need to look it up from the database. However, a setting can of course be null – in this case that just means that it is not specified for that user (or the general setting was null).

To determine whether a setting was loaded, I then used

if (!isset($settings[$key]))
{
$settings[$key] = doLookupSettingFromDatabase($key);
}

and then return the corresponding setting. This of course works whenever the setting is loaded – unless the setting is null. However, in this case, the array key was actually set – and the value of the key was set, just set to null. The isset() call in this case is a major fail.

In this setting it was a “simple” matter of replacing the isset()-call in the if-test with an array_key_exists()-call, which made the whole thing work as intended. You might also argue the case that I shouldn’t be using isset() in this case, and that array_key_exists() would be the intended function call (as the php docs suggests), but I don’t think that is correct.

I actually like to initialise variables that are later to be used, by assigning them an initial null value – and then if I want to see if I’ve set it to something meaningful, simply call

$variable = null;

{ … snippet of code that might change $variable … }

if ($variable !== null)
{
doSomething();
}

This has worked perfectly for me. I suspect the current isset() behaviour was implemented by someone that thought it was a great idea to implement the same logic in the isset() function. However – there are several situations where a variable would be assigned a null value, and calling isset() on that variable would then return false. In my opinion, a variable is only not isset() if unset() has been called on it. In all other cases, it is set, even if it is set to a null value.

There is of course no fix for this intended behaviour – as changing this isset() behaviour would cause a complete backwards compatibility break – so watch out for those isset() statements.

PS: This flawed logic is not applied to property_exists(). AFAIK, there is no “variable_exists” – this is what isset() is for, and the documentation for property_exists() actually mentions this. Fail x 2, but props for pointing it out in the manual.

PS2: This would probably be a good case to argue the undef data type.