99

I have several older applications that throw a lot of "xyz is undefined" and "undefined offset" messages when running on the E_NOTICE error level, because the existence of variables is not explicitly checked using isset() and consorts.

I am considering working through them to make them E_NOTICE compatible, as notices about missing variables or offsets can be lifesavers, there may be some minor performance improvements to be gained, and it's overall the cleaner way.

However, I don't like what inflicting hundreds of isset() empty() and array_key_exists() s does to my code. It gets bloated, becomes less readable, without gaining anything in terms of value or meaning.

How can I structure my code without an excess of variable checks, while also being E_NOTICE compatible?

Pekka
  • 442,112
  • 142
  • 972
  • 1,088
  • 6
    I agree completely. That's why I like Zend Framework so much, the request module is very good there. If I am working on some small app, I usually code some simple request class with magic methods __set and __get that works similar to ZF's request. That way I avoid all occurences of isset and empty in my code. That way all you need to use is either if (count($arr) > 0) on arrays before iterating over them and if (null !== $variable) at few critical places. It looks much cleaner. – Richard Knop Nov 05 '10 at 17:01

11 Answers11

130

For those interested, I have expanded this topic into a small article, which provides the below information in a somewhat better structured form: The Definitive Guide To PHP's isset And empty


IMHO you should think about not just making the app "E_NOTICE compatible", but restructuring the whole thing. Having hundreds of points in your code that regularly try to use non-existent variables sounds like a rather badly structured program. Trying to access non-existent variables should never ever happen, other languages balk at this at compile time. The fact that PHP allows you to do it doesn't mean you should.

These warnings are there to help you, not to annoy you. If you get a warning "You're trying to work with something that doesn't exist!", your reaction should be "Oops, my bad, let me fix that ASAP." How else are you going to tell the difference between "variables that work just fine undefined" and honestly wrong code that may lead to serious errors? This is also the reason why you always, always, develop with error reporting turned to 11 and keep plugging away at your code until not a single NOTICE is issued. Turning error reporting off is for production environments only, to avoid information leakage and provide a better user experience even in the face of buggy code.


To elaborate:

You will always need isset or empty somewhere in your code, the only way to reduce their occurrence is to initialize your variables properly. Depending on the situation there are different ways to do that:

Function arguments:

function foo ($bar, $baz = null) { ... }

There's no need to check whether $bar or $baz are set inside the function because you just set them, all you need to worry about is if their value evaluates to true or false (or whatever else).

Regular variables anywhere:

$foo = null;
$bar = $baz = 'default value';

Initialize your variables at the top of a block of code in which you're going to use them. This solves the !isset problem, ensures that your variables always have a known default value, gives the reader an idea of what the following code will work on and thereby also serves as a sort of self-documentation.

Arrays:

$defaults = array('foo' => false, 'bar' => true, 'baz' => 'default value');
$values = array_merge($defaults, $incoming_array);

The same thing as above, you're initializing the array with default values and overwrite them with actual values.

In the remaining cases, let's say a template where you're outputting values that may or may not be set by a controller, you'll just have to check:

<table>
    <?php if (!empty($foo) && is_array($foo)) : ?>
        <?php foreach ($foo as $bar) : ?>
            <tr>...</tr>
        <?php endforeach; ?>
    <?php else : ?>
        <tr><td>No Foo!</td></tr>
    <?php endif; ?>
</table>

If you find yourself regularly using array_key_exists, you should evaluate what you're using it for. The only time it makes a difference is here:

$array = array('key' => null);
isset($array['key']); // false
array_key_exists('key', $array); // true

As stated above though, if you're properly initializing your variables, you don't need to check if the key exists or not, because you know it does. If you're getting the array from an external source, the value will most likely not be null but '', 0, '0', false or something like it, i.e. a value you can evaluate with isset or empty, depending on your intent. If you regularly set an array key to null and want it to mean anything but false, i.e. if in the above example the differing results of isset and array_key_exists make a difference to your program logic, you should ask yourself why. The mere existence of a variable shouldn't be important, only its value should be of consequence. If the key is a true/false flag, then use true or false, not null. The only exception to this would be 3rd party libraries that want null to mean something, but since null is so hard to detect in PHP I have yet to find any library that does this.

deceze
  • 510,633
  • 85
  • 743
  • 889
  • 4
    True, but most failing access attempts are along the lines of `if ($array["xyz"])` instead of `isset()` or `array_key_exists()` which I find somewhat legitimate, certainly not structural problems (correct me if I'm mistaken). Adding `array_key_exists()` just looks like a terrible waste to me. – Pekka Dec 25 '09 at 06:14
  • 9
    I can't think of any case where I'd use `array_key_exists` instead of a simple `isset($array['key'])` or `!empty($array['key'])`. Sure, both add 7 or 8 characters to your code, but I'd hardly call that a problem. It also helps to clarify your code: `if (isset($array['key']))` means this variable is indeed optional and may be absent, whereas `if ($array['key'])` just means "if true". If you get a notice for the latter one, you know your logic is screwed somewhere. – deceze Dec 25 '09 at 07:15
  • 6
    I believe the difference between isset() and array_key_exists() is that the latter will return true if the value is NULL. isset() won't. – Htbaa Dec 25 '09 at 12:31
  • 1
    True, but I couldn't think of a sane use case where I need to differenciate between a non-existent variable and a set key whos value is null. If the value evaluates to FALSE the distinction should be without a difference. :) – deceze Dec 25 '09 at 13:10
  • 1
    Array keys are certainly more annoying than undefined variables. But if you're not sure whether an array contains a key or not, it means _either_ you didn't define the array yourself _or_ you're pulling it from a source that you don't control. Neither scenario should happen very often; and if it happens, you have every reason to check if the array contains what you think it does. It's a security measure IMO. – kijin Nov 03 '10 at 05:33
  • @deceze I didn't want to imply that you should check every key of every "3rd party array". Taking a diff would be OK; getting it as an object would be even better. I just wanted to expand on what you said. If there are hundreds of places in your code where it's unclear whether or not an array key exists, something is obviously wrong. Also, if you host it in one of those godforsaken shared hosts which have register_globals enabled, someone could inject array keys through a crafted querystring containing square brackets. None of this contradicts what you said. I was more like replying to @Pekka – kijin Nov 03 '10 at 06:07
  • @kijin Sorry, misunderstood the context of your comment… :) – deceze Nov 03 '10 at 06:10
  • 1
    @deceze: There are many, many sane use cases for the need to differentiate between `NULL` and other "falsy" values in arrays. One would be the direct mapping of an associative array to a relational database row (ex. MySQL, which makes the distinction between `NULL` and `""`). – FtDRbwLXw6 Jan 12 '12 at 15:52
  • *slow clap* Thanks for this bit of sanity. If you're going to use it but you didn't make, you should check to make sure you actually have it to use! – Slobaum Mar 22 '13 at 19:50
  • Also if you are counting up with `++` on an array element You wouldn't really need to initialize all array keys, if you set `$a=array()` at the start. example: https://stackoverflow.com/questions/44724471/initialize-a-php-variable-if-not-set?noredirect=1#comment76431675_44724471 I would appreciate an easier solution for this case still. – rubo77 Jun 23 '17 at 16:22
37

Just write a function for that. Something like:

function get_string($array, $index, $default = null) {
    if (isset($array[$index]) && strlen($value = trim($array[$index])) > 0) {
        return get_magic_quotes_gpc() ? stripslashes($value) : $value;
    } else {
        return $default;
    }
}

which you can use as

$username = get_string($_POST, 'username');

Do the same for trivial stuff like get_number(), get_boolean(), get_array() and so on.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • 5
    This looks good, and does magic_quotes checking as well. Nice! – Pekka Dec 25 '09 at 06:11
  • Great function! Thanks a lot for sharing. – Mike Moore Aug 10 '10 at 03:58
  • 3
    Notice that $_POST['something'] may return array, e.g. inputs with ``. This would cause error (as trim cannot be applied to arrays) using above code, in this case one should use `is_string` and possibly `strval`. This is not simply a case where one should use `get_array` either since user input (malicious) maybe anything and user input parser should never throw error anyway. – Ciantic Jul 10 '12 at 12:29
  • 1
    I use the same kind of function but defined as such: function get_value(&$item, $default = NULL) { return isset($item) ? $item : $default; } The advantage of this function is that you can call it with arrays, variables and objects. The drawback is that the $item gets initialized (to null) afterwards if it was not. – Mat Aug 13 '12 at 20:04
  • You should turn off magic quotes globally, instead of dealing with them in 1 function. There are plenty of sources on the internet explaining magic quotes. – Kayla Aug 15 '14 at 02:49
13

I believe one of the best ways of coping with this problem is by accessing values of GET and POST (COOKIE, SESSION, etc.) arrays through a class.

Create a class for each of those arrays and declare __get and __set methods (overloading). __get accepts one argument which will be the name of a value. This method should check this value in the corresponding global array, either using isset() or empty() and return the value if it exists or null (or some other default value) otherwise.

After that you can confidently access array values in this manner: $POST->username and do any validation if needed without using any isset()s or empty()s. If username does not exist in the corresponding global array then null will be returned, so no warnings or notices will be generated.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Jamol
  • 2,281
  • 2
  • 28
  • 28
  • 1
    This is a great idea, and something I'm ready to restructure code for. +1 – Pekka Dec 25 '09 at 15:10
  • Unfortunately you won't be able to make those instances superglobal unless you assign them to $_GET or $_POST which would be pretty ugly. But you could use static classes of course... – ThiefMaster Oct 24 '10 at 12:38
  • 1
    You can't use getters and setters on "static classes". and writing one class per variable is bad practice as it implies code duplication, which is bad. I don't think this solution is the most adequate. – Mat Aug 13 '12 at 20:06
  • A public static member of a class acts like a superglobal, ie: HTTP::$POST->username, where you instantiate HTTP::$POST at some point before its use, ie. Class HTTP { public static $POST = array();...}; HTTP::$POST = new someClass($_POST);... – velcrow Aug 19 '13 at 18:10
6

I don't mind using the array_key_exists() function. In fact, I prefer using this specific function rather than relying on hack functions which may change their behavior in the future like empty and isset (strikedthrough to avoid susceptibilities).


I do however, use a simple function that comes handy in this, and some other situations in dealing with array indexes:

function Value($array, $key, $default = false)
{
    if (is_array($array) === true)
    {
        settype($key, 'array');

        foreach ($key as $value)
        {
            if (array_key_exists($value, $array) === false)
            {
                return $default;
            }

            $array = $array[$value];
        }

        return $array;
    }

    return $default;
}

Let's say you've the following arrays:

$arr1 = array
(
    'xyz' => 'value'
);

$arr2 = array
(
    'x' => array
    (
        'y' => array
        (
            'z' => 'value',
        ),
    ),
);

How do you get the "value" out of the arrays? Simple:

Value($arr1, 'xyz', 'returns this if the index does not exist');
Value($arr2, array('x', 'y', 'z'), 'returns this if the index does not exist');

We already have uni and multi-dimensional arrays covered, what else can we possibly do?


Take the following piece of code for instance:

$url = 'https://stackoverflow.com/questions/1960509';
$domain = parse_url($url);

if (is_array($domain) === true)
{
    if (array_key_exists('host', $domain) === true)
    {
        $domain = $domain['host'];
    }

    else
    {
        $domain = 'N/A';
    }
}
else
{
    $domain = 'N/A';
}

Pretty boring isn't it? Here is another approach using the Value() function:

$url = 'https://stackoverflow.com/questions/1960509';
$domain = Value(parse_url($url), 'host', 'N/A');

As an additional example, take the RealIP() function for a test:

$ip = Value($_SERVER, 'HTTP_CLIENT_IP', Value($_SERVER, 'HTTP_X_FORWARDED_FOR', Value($_SERVER, 'REMOTE_ADDR')));

Neat, huh? ;)

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Alix Axel
  • 151,645
  • 95
  • 393
  • 500
  • 6
    "Relying on hack functions that may change their behavior in the future"?! Sorry, but that's about the most ridiculous thing I've heard all week. First of all, `isset` and `empty` are *language constructs*, not functions. Secondly, if **any** core library functions/language constructs change their behavior, you may or may not be screwed. What if `array_key_exists` changes its behavior? The answer is it won't, as long as you're using it as documented. And `isset` is documented to be used exactly so. Worst case functions are deprecated over a major release version or two. NIH syndrome is bad! – deceze Dec 25 '09 at 07:52
  • I'm sorry deceze, but first of all **hack** is in **italics** in case you didn't noticed. =) Secondly, you mean that one shouldn't rely on `array_key_exists()` to check if a **key exists** in an **array**?! `array_key_exists()` was **created exactly for that**, I rather rely on it for this purpose than `isset()` and specially `empty()` whose official description is: "determine whether a variable is empty", doesn't mentions anything if it actually exists. Your comment and down-vote is one of the most ridiculous I've witnessed all **month**. – Alix Axel Dec 25 '09 at 08:20
  • Also, take `$arr = array('xyz' => null)`; `isset($arr['xyz'])`? **Oops!** `empty($arr['xyz'])`? **Oops!**, `array_key_exists('xyz', $arr)` **Finally**, it works **like it should**. – Alix Axel Dec 25 '09 at 08:25
  • 3
    I'm saying `isset` and `empty` are no more or less reliable than `array_key_exists` and can do the exact same job. Your second, long-winded example can be written as `$domain = isset($domain['host']) ? $domain['host'] : 'N/A';` with just core language features, no extra function calls or declarations necessary (note that I do not necessarily advocate use of the ternary operator though ;o)). For ordinary scalar variables you'll still need to use `isset` or `empty`, and you can use them for arrays the exact same way. "Reliability" is a bad reason for not doing so. – deceze Dec 25 '09 at 08:28
  • Re: `isset` and `null`: In the limited case where you're just interested in the array key being set, go ahead and use `array_key_exists`. In 90%+ of all cases I dare say you're usually interested in the value not equalling `false` though, and `isset` is shorter in these cases. Also note that you'll need to take the extra step of confirming that the array is actually an array before you use `array_key_exists` on it, which makes it way more complicated for most cases. – deceze Dec 25 '09 at 08:38
  • @deceze: My answer was aimed at dealing with array indexes, not scalar values. I also think reliability isn't such a bad reason to be worried about (hell, this whole question is about reliability): `empty(0)` didn't become deprecated "over a major release version or two", it simple stopped working the way it did before in PHP 4. Same goes for `empty(new stdClass())` in PHP 5. – Alix Axel Dec 25 '09 at 08:51
  • 1
    You made your point, although I don't agree with most of the stuff you said. I think you got it way wrong on the 90%+ cases, for instance I use the value of "0" in hidden fields in forms all the time. Still I believe the solution I provided doesn't deserve the down-vote **and may well be of some use** to Pekka. – Alix Axel Dec 25 '09 at 08:59
  • If you're writing `empty(0)` you have bigger problems, and the behavior of `$x = 0; empty($x);` did not change AFAIA. `empty` on "empty" objects is another topic, but then again loads of things changed between PHP 4 and 5 with regard to objects. Furthermore the question targeted `isset`, `empty` and `array_key_exists`, not array indexes. My point being, creating long custom functions to deal with simple issues that are already solved in the language (if you'd take the time to understand how) is bad practice. – deceze Dec 25 '09 at 09:12
  • You seem to take it way too personally when I skip the basics, I know that `empty` is a language construct (irrelevant to this matter BTW) and therefore cannot accept `0`, `new stdClass()` or any other value other than a variable for that matter, I'm sorry if I assumed you would figure that out by yourself for the sake of the comment length. If you're not aware of the changes implied you should research (http://google.com/search?q=empty(0)) a little before you affirm that "isset and empty are no more or less reliable than array_key_exists". – Alix Axel Dec 25 '09 at 09:24
  • "creating functions to deal with issues that are solved": I can't access an index of an array returned by a function directly. I can't return a default value if any of the indexes doesn't exist in an array, or if the variable isn't an array, etc... – Alix Axel Dec 25 '09 at 09:28
  • "the question targeted isset, empty and array_key_exists, not array indexes" - I never said I had a one size fits all solution, what I said before was that "my answer was aimed at dealing with array indexes" mostly because I saw Pekka comment on BalusC similar answer and it seemed useful to him. – Alix Axel Dec 25 '09 at 09:31
  • I'm sorry if I'm trying to be very precise here, which I understood to be the important part of this exchange. `$x = 0; empty($x);` always worked the same, what changed was `$x = "0"; empty($x);`, and it changed between PHP 3 and 4, not 4 and 5. Sorry if the downvote bothers you, but I'm calling it as I see it: Long custom functions for basics like variable handling is bad practice. – deceze Dec 25 '09 at 09:34
  • **A word to the wise is sufficient.** The down-vote doesn't bothers me a bit, what bothers me is your stubbornness... Both `$x = 0; empty($x);` and `$x = '0'; empty($x);` changed their behavior in PHP 4: "stopped working the way it did before in **a comma is missing - OHH!!** PHP 4", otherwise (**IF I meant from PHP 4 to 5**) I would've joined both (I'm explicitly referring to the "`empty(new stdClass())` in PHP 5") statements. Again, pretty obvious IMO. – Alix Axel Dec 25 '09 at 10:35
  • 2
    While @deceze has a point with the custom functions - I usually take the same stance - the value() approach looks interesting enough that I will be taking a look at it. I think the answer and the followup will enable everybody who stumbles upon it later to make up their own mind. +1. – Pekka Dec 25 '09 at 15:27
  • Nice function and for what its worth I agree with Alix that this is a handy function that saves a lot of typing and saves a lot of time when handling multi-arrays which I deal with constantly. +1 and thx. – Erik Čerpnjak Jan 22 '16 at 10:31
4

Welcome to null coalescing operator (PHP >= 7.0.1):

$field = $_GET['field'] ?? null;

PHP says:

The null coalescing operator (??) has been added as syntactic sugar for the common case of needing to use a ternary in conjunction with isset(). It returns its first operand if it exists and is not NULL; otherwise it returns its second operand.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Alexandre Thebaldi
  • 4,546
  • 6
  • 41
  • 55
3

I'm here with you. But PHP designers has made a lot more worse mistakes than that. Short of defining a custom function for any value reading, there isn't any way around it.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
vava
  • 24,851
  • 11
  • 64
  • 79
  • 1
    isset() stuff. Making everything null by default would save a lot of troubles. – vava Dec 25 '09 at 14:07
  • 2
    And what is this 'everything'? It would seem like a waste for PHP to have to imagine every conceivable variable name and set each to NULL just so a lazy developer can avoid typing 5 characters. – Lotus Notes May 19 '10 at 17:42
  • 5
    @Byron, look, it is really simple, a lot of other languages do that, Ruby and Perl as few examples. VM knows if variable was used before or not, doesn't it? It always can return null instead of failing with or without error message. And this is not about lousy 5 characters, it's about writing `params["width"] = params["width"] || 5` to set defaults instead of all that nonsense with `isset()` calls. – vava May 20 '10 at 03:12
  • 3
    Sorry for resurrecting an old thread. Two of PHP's worst mistakes were `register_globals` and `magic_quotes`. The problems these foster make uninitialized variables look almost harmless by comparison. – staticsan Nov 21 '10 at 22:40
3

I use these functions

function load(&$var) { return isset($var) ? $var : null; }
function POST($var) { return isset($_POST[$var]) ? $_POST[$var] : null; }

Examples

$y = load($x); // null, no notice

// this attitude is both readable and comfortable
if($login=POST("login") and $pass=POST("pass")) { // really =, not ==
  // executes only if both login and pass were in POST
  // stored in $login and $pass variables
  $authorized = $login=="root" && md5($pass)=="f65b2a087755c68586568531ad8288b4";
}
Jan Turoň
  • 31,451
  • 23
  • 125
  • 169
  • 2
    I use this too but remember that in some case, your variables will be initialized automatically: eg load($array['FOO']) would create a FOO key in $array. – Mat Aug 13 '12 at 20:08
1

Make a function which returns false if not set, and, if specified, false if empty. If valid it returns the variable. You can add more options as seen in the code below:

<?php
function isset_globals($method, $name, $option = "") {
    if (isset($method[$name])) {    // Check if such a variable
        if ($option === "empty" && empty($method[$name])) { return false; } // Check if empty 
        if ($option === "stringLength" && strlen($method[$name])) { return strlen($method[$name]); }    // Check length of string -- used when checking length of textareas
        return ($method[$name]);
    } else { return false; }
}

if (!isset_globals("$_post", "input_name", "empty")) {
    echo "invalid";
} else {
    /* You are safe to access the variable without worrying about errors! */
    echo "you uploaded: " . $_POST["input_name"];
}
?>
dragonfire
  • 407
  • 7
  • 16
0

I'm not sure what your definition of readability is, but proper use of empty(), isset() and try/throw/catch blocks, is pretty important to the whole process.

If your E_NOTICE is coming from $_GET or $_POST, then they should be checked against empty() right along with all the other security checks that that data should have to pass.

If it's coming from external feeds or libraries, it should be wrapped in try/catch.

If it's coming from the database, $db_num_rows() or its equivalent should be checked.

If it's coming from internal variables, they should be properly initialized. Often, these types of notices come from assigning a new variable to the return of a function that returns FALSE on a failure. Those should be wrapped in a test that, in the event of a failure, can either assign the variable an acceptable default value that the code can handle, or throwing an exception that the code can handle.

These things make the code longer, add extra blocks, and add extra tests, but I disagree with you in that I think they most definitely add extra value.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Mlutz
  • 9
  • 1
0

Software does not magically run by the grace of god. If you are expecting something that is missing, you need to properly handle it.

If you ignore it, you are probably creating security holes in your applications. In static languages accessing a non-defined variable it is just not possible. It won't simply compile or crash your application if it's null.

Furthermore, it makes your application unmaintainable, and you are going to go mad when unexpected things happen. Language strictness is a must and PHP, by design, is wrong in so many aspects. It will make you a bad programmer if you are not aware.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
knoopx
  • 17,089
  • 7
  • 36
  • 41
  • I am well aware of the shortfalls of PHP. As I pointed out in the question, I am talking about the overhauling of older projects. – Pekka Dec 25 '09 at 15:13
  • Agreed. Being a ling-time PHP developer it is quite difficult for me to venture into new languages like Java where you need to declare everything. – Dzhuneyt Sep 07 '12 at 09:38
-2

What about using the @ operator?

For example:

if(@$foo) { /* Do something */ }

You may say this is bad because you have no control of what happens "inside" $foo (if it was a function call that contains a PHP error for example), but if you only use this technique for variables, this is equivalent to:

if(isset($foo) && $foo) { /* ... */ }
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Mat
  • 1,022
  • 2
  • 9
  • 24
  • `if(isset($foo))` is enough actually. It will return `TRUE` if the expression evaluates to `TRUE`. – Dzhuneyt Sep 07 '12 at 09:37
  • 2
    @ColorWP.com it will also return true if the expression evaluates to false. – Jon Hulka Nov 22 '12 at 10:11
  • You should only use the @ parameter (to ignore the notice) on code that is not really under further development, or on one-time code or a quick-fix on existing projects, that you don't want to show anybody else. But it is a common workaround for a quick hack. – rubo77 Jun 23 '17 at 16:14