tags:

views:

77

answers:

5

I have a foreach loop, that will loop through an array, but the array may not exist depending on the logic of this particular application.

My question relates to I guess best practices, for example, is it ok to do this:

if (isset($array))
{

    foreach($array as $something)
    {
        //do something
    }
}

It seems messy to me, but in this instance if I dont do it, it errors on the foreach. should I pass an empty array?? I haven't posted specific code because its a general question about handling variables that may or may not be set.

+1  A: 

Try:

 if(!empty($array))
 {
     foreach($array as $row)
     {
         // do something
     }
 }
Manie
Thats just as bad as `isset` because if there is a value set and its not an array the coede will proceed and possibly produce un predictable results.
prodigitalson
@prodigitalson I'd have to say that if the variable possibly contains an array, or possibly contains something else like a string, or possibly doesn't exist.... you're doing something wrong.
Stephen
yeah that's what I thought about isset/empty you really need to understand their precise purpose.. but also, like you said the variable could contain something different. what do you call that where you ensure that data is the correct type? I thought 'type checking' or something? when people say PHP is a loosley typed language, is that what it means? in the sense that it will allow you to interchange data types whereas some other languages require you to declare them etc
Tim
I agree with @Stephen. A simple `!empty` should be the best way to go in most cases, unless `$array` can legitimately be anything but an array. Testing for `is_array` as well just silently suppresses errors you'd otherwise easily find.
deceze
A: 

That's not messy at all. In fact, it's best practice. If I had to point out anything messy it would be the use of Allman brace style, but that's personal preference. (I'm a 1TBS kind of guy) ;)

I'll usually do this in all of my class methods:

public function do_stuff ($param = NULL) {
    if (!empty($param)) {
        // do stuff
    }
}

A word on empty(). There are cases where isset is preferable, but empty works if the variable is not set, OR if it contains an "empty" value like an empty string or the number 0.

Stephen
ah man, dont get me started on brace styling, I just switched to this one because I read (I think) the Codeigniter guidelines who recommended it lol
Tim
@user270797 That's funny! A coworker of mine just wrote a few regex find-and-replace calls to parse the entire codeigniter core files and convert them to 1TBS because the allman style was so annoying!
Stephen
no problamo! comment deleted.
Stephen
yeah I know, I hated it but I thought well if that's what they're recommending then I should try and get used to it
Tim
Actually, those recommendations in the user guide are only for people trying to write code for the codeignigter project, like helpers or core updates. Check out http://en.wikipedia.org/wiki/Indent_style for other variants. I wouldn't use Allman if it was uncomfortable. Well, I wouldn't use Allman anyway, but I already said that. :)
Stephen
is it really just a preference thing? why would they recommend it for the core? it's the same as camelCase vs under_scores - I prefer the latter, originally started out with camelCase when I was working in VB but since learning PHP underscores seem more natural
Tim
It is most definitely a preference thing. For example, look at the CakePHP project, another great framework. They use 1TBS, not Allman. And they use CamelCase for class names.
Stephen
Good!, I can go back to the way I like it! lol I prefer 1TBS also
Tim
Yes! Excellent! Allman style should die.
Stephen
A: 

If you pass an empty array to foreach then it is fine but if you pass a array variable that is not initialized then it will produce error.

It will work when array is empty or even not initialized.

if( !empty($array) && is_array($array) ) {
   foreach(...)
}
NAVEED
This will generate warnings if `$array` is not empty but not an array.
NullUserException
@NullUserException: Yes you are right but I thought when you are using a variable named "array" then it may be only array or you forgot to initialize according to question. Anyway **is_array** condition is ok for this.
NAVEED
@NAVEED There's nothing stopping me from saying `$array = null`, is there?
NullUserException
@NullUserException: Is it produce errors when your **$array = null** ?
NAVEED
@NAVEED It doesn't now, but your previous answer would give errors when `$array = 'string';`
NullUserException
+8  A: 

Just to note: here is the 'safest' way.

if (isset($array) && is_array($array)) {
    foreach ($array as $item) {
        // ...
    }
}
strager
That's a good approach. +1
Stephen
A: 

I would say it is good practice to have a 'boolean' other value that is set as 0 (PHP's false) to start, and any time some function adds to this array, add +1 to the boolean, so you'll have a definite way to know if you should mess with the array or not?

That's the approach I would take in an object oriented language, in PHP it could be messier, but still I find it best to have a deliberate variable keeping track, rather than try to analyze the array itself. Ideally if this variable is always an array, set the first value as 0, and use it as the flag:

 <?PHP
 //somewhere in initialization
 $myArray[0] = 0;
 ...
 //somewhere inside an if statement that fills up the array
 $myArray[0]++;
 $myArray[1] = someValue;

 //somewhere later in the game:
 if($myArray[0] > 0){          //check if this array should be processed or not
      foreach($myArray as $row){     //start up the for loop
           if(! $skippedFirstRow){   //$skippedFirstRow will be false the first try
                $skippedFirstRow++;  //now $skippedFirstRow will be true
                continue;            //this will skip to the next iteration of the loop
           }
           //process remaining rows - nothing will happen here for that first placeholder row
      }
 }
 ?>
Alex Gosselin
Just a note - you should try to avoid doing this, the above method is a last resort, cause it definitely still is messy.
Alex Gosselin
He's asking what the best practice is for checking to see if a variable is an array. I'm not sure under what scenario your code makes sense. 0 isn't PHP's false: false (or FALSE) is PHP's false, and why would you ever set the first element in an otherwise normal array to false? What is this code trying to accomplish?
Mark Trapp
ok, a little rusty on PHP's true/false, regardless this still works.If you read the question again though, he's talking about an array that may or may not exist, and using the existence of this array for a logical decision. I would say that for any major code it's not good practice to use the existence of the array for program logic in any case, and you should make a variable to track it instead. In this case I chose to store the true/false as a 1/0 in the array itself.Read over the code again, you should be able to figure out how it works, I commented thoroughly.
Alex Gosselin