!empty() check before a foreach statement

Question

Is this the best way to prevent that foreach loop from happening if $terms doesn't exist?

$terms = get_the_terms( $postid, 'fecha' );
if(!empty($terms)) {
 foreach($terms as $term) {
 $myterm = $term->slug;
 if(in_array($myterm, $queried_terms)) {continue;}
 $queried_terms[] = $myterm;
 }
}

Answer 1

The loop won't be executed if $terms is false/null/an empty array.

As such: you don't need the empty check. It doesn't hurt, but it's simply not required.

If $terms is sometimes not an array

You can just cast it to an array before use:

$terms = get_the_terms( $postid, 'fecha' );
foreach((array) $terms as $term) {
 $myterm = $term->slug;
 if(in_array($myterm, $queried_terms)) {
 continue;
 }
 $queried_terms[] = $myterm;
}

The code would work with or without casting to an array - the only difference is that if $terms is not an array, a notice would be thrown without it.

if this is part of another function

you can simply return-early to keep your code concise:

function foo() {
 $terms = get_the_terms( $postid, 'fecha' );
 if (!$terms) {
 return array();
 }
 $queried_terms = array();
 foreach($terms as $term) {
 $myterm = $term->slug;
 if(in_array($myterm, $queried_terms)) {
 continue;
 }
 $queried_terms[] = $myterm;
 }
 return $queried_terms;
}

But, any way you slice it - the empty check isn't strictly required, and deeper code nesting is something to be aware of and avoid.

• mseancole: +1 However I question the wisdom in typecasting a variable like that. A variable, in my opinion, should not change types so frequently that you would not know its type. If you find yourself doing this then you should probably implement new variables into your program for those other types. At most my variables are either a boolean or of the desired type so that I can check if it did indeed pass some check or has the value I am looking for. Just my two cents :)
• AD7six: if terms is either an array or falsey - it's fine and appropriate.
• mseancole: It still should be used with caution. That's all I was trying to point out. I personally wouldn't use it, but I know many programmers do. That's why I was trying to explain that it shouldn't be used to force your code to work.
• AD7six: I agree with your sentiment, but get_the_terms sounds like wordpress. That probably gives some indication of expectations and the environment (notices ignored, because otherwise you'd drown in them). Casting to an array only avoids a notice and only "If $terms is sometimes not an array" - IME you can't trust anything coming out of wordpress to consistently be what you expect. Casting variables in and of itself is not a bad thing - especially if you work in a type-sensitive system (e.g. mongo as db, json as output).

Answer 2

$terms = get_the_terms( $postid, 'fecha' );
if(is_array($terms)) {
 foreach($terms as $term) {
 $myterm = $term->slug;
 if(in_array($myterm, $queried_terms)) {continue;}
 $queried_terms[] = $myterm;
 }
}

I always use this method and never get problem