8

Yesterday, I posted an answer to a question that included several (unknown to me at the time) very bad code examples. Since then, I've been looking at my fundamental knowledge of PHP that allowed me to think that such code is possible. This brings me to a question that I can't seem to find an answer to:

If I want to check for whether or not a variable has anything set, is it a valid practice to not use isset() or another helper function? here's a "for instance":

if($not_set){
    //do something
} else {
    //do something else
}

Rather than...

if(isset($not_set)){
    //do something
} else {
    //do something else
}

From the name of the variable, you can see that this variable is not set. Therefore the conditional would be false and the else portion would run. Up until now I have been using this practice, but after the posts yesterday, I now have an inkling that this is wrong.

Here's why I thought that it would be an ok practice to leave out the isset() function above. From PHP manual:

The if construct is one of the most important features of many languages, PHP included. It allows for conditional execution of code fragments. PHP features an if structure that is similar to that of C:

if (expr) statement

As described in the section about expressions, expression is evaluated to its Boolean value. If expression evaluates to TRUE, PHP will execute statement, and if it evaluates to FALSE - it'll ignore it. More information about what values evaluate to FALSE can be found in the 'Converting to boolean' section.

And from the 'Converting to boolean section':

When converting to boolean , the following values are considered FALSE:

... * the special type NULL (including unset variables)

Why would the manual go out of its way to state that unset variables are included if this is a bad practice? If it's unset, it gets converted to NULL and therefore is evaluated properly by the conditional. Using isset() will find the same result, but will take extra cycles to do so.

Have I been wrong this whole time, and if so, why? (And just how bad it is, maybe?)

halfer
  • 19,824
  • 17
  • 99
  • 186
TCCV
  • 3,142
  • 4
  • 25
  • 30
  • Well, your approach *will* catch unset variables, but it will also catch other variables which *are* set. So it's not really a test for variables being set – Gareth Jul 20 '10 at 23:23
  • Also see http://stackoverflow.com/questions/1960509/isset-and-empty-make-code-ugly – deceze Jul 20 '10 at 23:25
  • Hey deceze! Thanks for everything yesterday - huge leap in my PHP understanding (as you can see). I didn't see that one when I was searching, but that's basically the same thing I was thinking. – TCCV Jul 20 '10 at 23:31
  • I just want to reiterate one of my comments in the thread linked to above: `isset` helps to clarify your code and aid debugging: `if (isset($var))` means "this variable is optional and may be absent", and the rest of your code needs to treat it as such. `if ($var)` just means "if true". If PHP throws a warning for the latter one, you know your code is screwed somewhere. And you always need to program with error reporting on full blast for this reason. – deceze Jul 21 '10 at 00:08

5 Answers5

11

If the variable is not set you get a Notice. If you use isset() you don't get a notice. So from an error reporting point of view, using isset() is better :)

Example:

error_reporting(E_ALL);   
if($a) {
   echo 'foo';
}

gives

Notice: Undefined variable: a in /Users/kling/test on line 5

whereas

error_reporting(E_ALL);
if(isset($a)) {
   echo 'foo';
}

does not output anything.


The bottom line: If code quality is important to you, use isset().

Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
  • From *error reporting* point of view, using isset is worse. Obviously, when you are trying to access a variable that doesn't exist, it's an error. And isset effectively works as an error suppression operator. The bottom line: If code quality is important to you, use isset() with caution, basically with only outside variables you have no control for. – Your Common Sense Sep 05 '22 at 05:42
7

It's okay but not good practice to use if to check for a set variable. Two reasons off the top of my head:

  1. Using isset makes the intent clear - that you're checking whether the variable is set, and not instead checking whether a condition is true.
  2. if ($not_set) will evaluate to false when $not_set is actually set but is equal to boolean false.
casablanca
  • 69,683
  • 7
  • 133
  • 150
6

You will run in to problems if your variable is set, but evaluates to FALSE, like the following:

  • the boolean FALSE itself
  • the integer 0 (zero)
  • the float 0.0 (zero)
  • the empty string, and the string "0"
  • an array with zero elements
  • an object with zero member variables (PHP 4 only)
  • the special type NULL (including unset variables)
  • SimpleXML objects created from empty tags

Taken from the PHP manual.

Basically, using isset() is showing that you are explicitly checking if a variable exists and is not NULL, while the structure of your if statement only checks if the variable is true. It is more clear and less error-prone.

Jamison Dance
  • 19,896
  • 25
  • 97
  • 99
4

It is a common practise, but is not good -- you should always use isset!

If your $not_set is set, and is a bool with the value false, your "test" will fail!

Michael Mrozek
  • 169,610
  • 28
  • 168
  • 175
Andreas Rehm
  • 2,222
  • 17
  • 20
1

isset works as a guard preventing you from using variables that do not actually exist.
if (isset($foo)) and if ($foo) do not mean the same thing. isset just tells you if the variable actually exists and if it's okay to use it, it does not evaluate the value of the variable itself*.

Hence, you should usually use one of these two patterns:

If the variable is sure to exist and you just want to check its value:

if ($foo == 'bar')

If the variable may or may not exist, and you want to check its value:

if (isset($foo) && $foo == 'bar')

If you're just interested that a variable is set and evaluates to true, i.e. if ($foo), you can use empty:

if (isset($foo) && $foo)
// is the same as
if (!empty($foo))

* it does check for null, where null is as good as not being set at all

deceze
  • 510,633
  • 85
  • 743
  • 889