40

I am working through some code done by a previous developer. I'm pretty new to PHP so I am wondering if there is any well known pattern or solution to this problem.

Basically the original author does not check any array indexes before he tries to use them. I know I can use isset() to check each before it is used, but right now there are hundreds of lines where these errors are appearing. Before I put on some music and start slamming my head into my keyboard I want to make sure there is not some nice shortcut for handling this. Here is a typical section of code I'm looking at:

    /* snip */
"text" => $link . $top_pick_marker . $output['author'] . " " .  " " . 
                              $output['new_icon'] . $output['rec_labels'] . "   " 
                    . $output['admin_link']
                    . $output['alternate_title'] 
                    . $output['access_info'] 
                    . $output['description'] 
                    . $output['url']
                    . $output['subject_terms'] 
                    . $output['form_subdivisions'] 
                    . $output['dates_of_coverage']  
                    . $output['update_frequency']  
                    . $output['place_terms'],
    /* snip */

I know I can use isset() here for each item. I would have to rearrange things a bit and remove all the concatenation as it is now. Is there any other easy way to do this or am I just stuck with it?

Dharman
  • 30,962
  • 25
  • 85
  • 135
  • 10
    +1 - This is actually a great question. In the "old days" of PHP, these E_NOTICE errors weren't thrown, and referencing uninitialized array indexes was very common. Obviously this is a bad habit, but it's easy to do with the loose typing of PHP. The E_NOTICES are a good tool to help tighten your code now. However, I find the sheer tedium of having to call `isset()` or `empty()` on all checks involving arrays quite mind-numbing. – zombat Jul 28 '09 at 18:10
  • @$output['author'] by-passes the check and uses the data or null if its not set. – ppostma1 Mar 12 '19 at 21:46

10 Answers10

26

Figure out what keys are in the $output array, and fill the missing ones in with empty strings.

$keys = array_keys($output);
$desired_keys = array('author', 'new_icon', 'admin_link', 'etc.');

foreach($desired_keys as $desired_key){
   if(in_array($desired_key, $keys)) continue;  // already set
   $output[$desired_key] = '';
}
SquareRootOf2
  • 393
  • 2
  • 5
  • This is a good solution for a large list of index keys. I would use something like this for the poster's problem. – zombat Jul 28 '09 at 18:16
  • I prefer to set the defaults to **null** – dejjub-AIS Jun 04 '12 at 11:36
  • This is great except if your `$output` array is multi-dimensional. – cdmo Sep 17 '13 at 14:29
  • @cdmo in that case you might benefit from something like Laravel's `array_set($array, 'foo.bar.baz', $value)` helper. http://laravel.com/docs/4.2/helpers – Don McCurdy Nov 03 '14 at 02:26
  • It would be more appropriate to use array_key_exists instead of in_array See http://php.net/manual/en/function.array-key-exists.php – Chiwda Feb 17 '16 at 15:10
12

You can use isset() without losing the concatenation:

//snip
$str = 'something'
 . ( isset($output['alternate_title']) ? $output['alternate_title'] : '' )
 . ( isset($output['access_info']) ? $output['access_info'] : '' )
 . //etc.

You could also write a function to return the string if it is set - this probably isn't very efficient:

function getIfSet(& $var) {
    if (isset($var)) {
        return $var;
    }
    return null;
}

$str = getIfSet($output['alternate_title']) . getIfSet($output['access_info']) //etc

You won't get a notice because the variable is passed by reference.

Tom Haigh
  • 57,217
  • 21
  • 114
  • 142
  • He did say he didn't want to use `isset`, but this is still the "neatest" way to do it with `isset`. – MitMaro Jul 28 '09 at 17:45
  • Also this way wouldn't be hard to implement since you could probably use a regex search and replace to do it automatically. – MitMaro Jul 28 '09 at 17:47
  • @MitMaro - agreed. I find the ternary operator very handy for handling this problem for string output or concatenation. For a small number of array keys, this method works well. – zombat Jul 28 '09 at 18:15
8

A variation on SquareRootOf2's answer, but this should be placed before the first use of the $output variable:

$keys = array('key1', 'key2', 'etc');
$output = array_fill_keys($keys, '');
Sterling Hirsh
  • 111
  • 3
  • I like the idea of "not using the loop" and prefer to use PHP functions. My suggestion is to use `array_merge()` like so `$output = array_merge($incomplete_output, $output)` ? +1 from me – Prabowo Murti Mar 02 '18 at 03:43
6

If you are maintaining old code, you probably cannot aim for "the best possible code ever"... That's one case when, in my opinion, you could lower the error_reporting level.

These "undefined index" should only be Notices ; so, you could set the error_reporting level to exclude notices.

One solution is with the error_reporting function, like this :

// Report all errors except E_NOTICE
error_reporting(E_ALL ^ E_NOTICE);

The good thing with this solution is you can set it to exclude notices only when it's necessary (say, for instance, if there is only one or two files with that kind of code)

One other solution would be to set this in php.ini (might not be such a good idea if you are working on several applications, though, as it could mask useful notices ) ; see error_reporting in php.ini.

But I insist : this is acceptable only because you are maintaining an old application -- you should not do that when developping new code !

Pascal MARTIN
  • 395,085
  • 80
  • 655
  • 663
  • 1
    Personally I don't think you should ever hide any 'errors' even in old code. More then likely the errors will be hidden in the old code and there will be no incentive to fix them. – MitMaro Jul 28 '09 at 17:51
  • I kind of agree, and I don't like doing that either ; but, sometimes, there is really no other way, if you want to be able to work (at least if you don't have enough time to correct those) ; especially when the application has been developped by someone who didn't know what notices are and to both enable and avoid them... – Pascal MARTIN Jul 28 '09 at 17:55
4

Set each index in the array at the beginning (or before the $output array is used) would probably be the easiest solution for your case.

Example

$output['admin_link'] = ""
$output['alternate_title'] = ""
$output['access_info'] = ""
$output['description'] = ""
$output['url'] = ""

Also not really relevant for your case but where you said you were new to PHP and this is not really immediately obvious isset() can take multiple arguments. So in stead of this:

if(isset($var1) && isset($var2) && isset($var3) ...){
    // all are set
}

You can do:

if(isset($var1, $var2, $var3)){
   // all are set 
}
MitMaro
  • 5,607
  • 6
  • 28
  • 52
4

A short solution is this (PHP 5.3+):

$output['alternate_title'] = $output['alternate_title'] ?:'';

You get either the value of the variable, if it doesn't evaluate to false, or the false expression. (The one after the ':')

Using the ternary Operator, without the "if true" parameter, will return the result of the test expression (The first one ) Since undefined evaluates to false, the false expression will be returned.

In PHP 7 there is the slightly more elegant Null coalescing operator:

$output['alternate_title'] = $output['alternate_title'] ?? '';

(It would be nice with a default assignment operator like '?=')

Simon Rigét
  • 2,757
  • 5
  • 30
  • 33
  • `?:` is DEFINITELY NOT the solution to receiving notices for undeclared variables. And yes, modern PHP has a null coalescing assignment operator -- `??=`. – mickmackusa Jan 18 '21 at 20:11
0

Same idea as Michael Waterfall

From CodeIgniter 

// Lets you determine whether an array index is set and whether it has a value.
// If the element is empty it returns FALSE (or whatever you specify as the default value.)
function element($item, $array, $default = FALSE)
{
    if ( ! isset($array[$item]) OR $array[$item] == "")
    {
        return $default;
    }

    return $array[$item];
}
Jerome Jaglale
  • 1,863
  • 18
  • 22
  • `! isset($array[$item]) OR $array[$item] == ""` is `empty()`, and as I said on Jodyshop's answer, this may damage data unintentionally. – mickmackusa Jan 20 '21 at 21:47
  • @mickmackusa, how so? Could you link to some relevant resource? – Jerome Jaglale Jan 23 '21 at 08:26
  • https://stackoverflow.com/a/1219550/2943403 , https://stackoverflow.com/q/7191626/2943403 , https://stackoverflow.com/q/20582962/2943403 , and most importantly: https://stackoverflow.com/q/4559925/2943403 – mickmackusa Jan 23 '21 at 08:54
  • @mickmackusa, these links are "isset vs empty" discussions. I don't see any mention of "unintentional data damage". Could you give an example? – Jerome Jaglale Jan 23 '21 at 09:30
  • Ah, okay, I misunderstood your query. Alas, from PHP8 your technique does differ from previous versions because [the rules have been changed when comparing strings and integers](https://wiki.php.net/rfc/string_to_number_comparison) demo: https://3v4l.org/kv6MN However, the loose comparison problem when checking the non-integer scalar input against an empty string remains inaccurate even in PHP8. demos: [checking `false` value](https://3v4l.org/18Y0q) and [checking `null` value](https://3v4l.org/j4iDh) Back when you posted, all falsey values would become `$default` (e.g. empty array) – mickmackusa Jan 23 '21 at 09:52
-1

In my case, I get it worked by defining the default value if the submitted data is empty. Here is what I finally did (using PHP7.3.5):

if(empty($_POST['auto'])){ $_POST['auto'] = ""; }

Jodyshop
  • 656
  • 8
  • 12
  • `empty()` may damage legitimate data because it consumes all falsey values. This is not an appropriate technique for all circumstances. – mickmackusa Jan 18 '21 at 20:13
-2

You could try using a nice little function that will return the value if it exists or an empty string if not. This is what I use:

function arrayValueForKey($arrayName, $key) {
   if (isset($GLOBALS[$arrayName]) && isset($GLOBALS[$arrayName][$key])) {
      return $GLOBALS[$variable][$key];
   } else {
      return '';
   }
}

Then you can use it like this:

echo ' Values: ' . arrayValueForKey('output', 'admin_link') 
                 . arrayValueForKey('output', 'update_frequency');

And it won't throw up any errors!

Hope this helps!

Michael Waterfall
  • 20,497
  • 27
  • 111
  • 168
  • Why the use of `GLOBALS` instead of just using the arguments? – grantwparks Sep 25 '09 at 20:00
  • Using globals would be a good way to do this as it avoids a 3rd argument in the function, keeping things clean and simple to use again and again. Admittedly it won't work when using OO PHP, but for simple things it'll work great with not much more code than would normally be needed to access the array directly. – Michael Waterfall Sep 26 '09 at 16:32
  • It is not necessary to make to separate `isset()` calls. Not only can `isset()` receive multiple arguments, there is no need to check the parent element before the child element. – mickmackusa Jan 18 '21 at 20:14
-4
foreach($i=0; $i<10; $i++){
    $v = @(array)$v;   
    // this could help defining $v as an array. 
    //@ is to supress undefined variable $v

    array_push($v, $i);
}
jenson
  • 1
  • Whenever I see a script that uses the STFU operator, I instantly know that I am looking at non-professional, poorly maintained code. – mickmackusa Jan 18 '21 at 20:15