views:

166

answers:

5

Hello,

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.

Can someone please enlighten me as to whether I've been wrong this whole time and why? (And just how bad it is, maybe?)

Thanks, SO, you never disappoint.

Edit: Thanks everyone (and that was quick). I honestly think all the answers so far are great and don't know which to pick for the answer... If yours isn't picked, I will still upvote :o)

+10  A: 

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
+4  A: 

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!

Andreas Rehm
+7  A: 

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
+1 for mentioning intention.
Felix Kling
+4  A: 

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.

Jergason
`isset` checks if a variable **exists** (*is set*), and is not `null`. The *is set* part is a lot more important than the *not `null`* part.
deceze
Thanks for the clarification. Edited to make add that in.
Jergason
+1  A: 

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