2

I've taken over a larger code base, that has several warnings and errors.

I keep running into statements like this:

foreach( $object->keys => $key ){
  ...
}

Where I can see in the error log, that $object is null.

What is the simplest way, tot check that above-written statement won't break?

To be 100% sure, I would do something like this:

if( isset( $object ) && is_array( $object->keys ) ){
  foreach( $object->keys => $key ){
    ...
  }
}

But the readability decreases significantly.

I could also make a helper function, that did it, like so:

if( myHelper::canBeForeached( $object, 'keys' ) ){ // naming would obviously be changed
  foreach( $object->keys => $key ){
    ...
  }
}

But that too seems bad.

What is the simplest/prettiest way to achieve this, to also maintain readability of the code?

Zeth
  • 2,273
  • 4
  • 43
  • 91
  • 1
    Perhaps something like this, https://stackoverflow.com/a/42667976/296555, but the better solution is to fix the underlying code issues that produce different data types. This may be a big undertaking on large projects though. – waterloomatt Feb 02 '22 at 17:44
  • ```foreach( $object->keys => $key )``` is a syntax error? Did you mean ```as $key```? In any case, the short way (that feels a bit dirty) is null coalescing, ie. ```foreach($object->keys ?? [] as $key) {```, ie. if `$object` or `keys` is not defined, you provide `[]` an empty array, which obviously just won't be iterated. This assumes that if `$object` exists and `->keys` exists, `keys` will be iterable. – Markus AO Feb 02 '22 at 18:25

1 Answers1

1

There are a number of ways to approach this. Here's the shortest approach.

If you use the ?? null-coalescing operator for default value, in the event that either $object or ->keys is undefined, then [] or an empty array will be passed for iteration, and obviously nothing happens. This is one way to avoid wrapping your foreach loop inside a condition.

foreach($object->keys ?? [] as $key) {
    // If $object or ->keys is undefined, nothing happens.
}

Now, some may object that this is unclear or unreadable; but at least it's not a ton of boilerplate, and it's very quick to add in. This approach assumes that if $object exists and has the property keys, then keys will be iterable. If not, it will result in a warning on must be of type array|object.

Other than that, you'd have to wrap all your loops with a condition check, isset($object), or alternatively, gettype($object) === 'object' if you want to ensure that it's an object. For some reason, is_object() will give a warning for an undefined variable. Again, is_null() returns true if the variable is undefined, essentially behaving like !isset() when negated.

As noted above, if your legacy code has objects with plural-named properties that are not iterable but also not null (e.g. ->keys = 'duck'), you'll have to do a check on your $object->keys with is_iterable() before it enters the loop for an attempt at iteration.

If you pass an undefined object into is_iterable, it's a warning again. You can, however, use the power of null-coalescing once more, where if either $object or ->keys is undefined, the check below reads as is_iterable(false) (or any other non-iterable value to evaluate):

if(is_iterable($object->keys ?? false)) { 
    foreach($object->keys as $key) {
    //...
    }
}

...and that could be the sole condition that wraps your loop.

P.S. On the possible solutions in OP: A helper function results in warnings if used with an undefined variable. Also, your if( isset( $object ) && is_array( $object->keys ) ) will result in a warning if object is defined but doesn't have the ->keys property.

Markus AO
  • 4,771
  • 2
  • 18
  • 29
  • Wrote a related answer on using null-coalescence to provide default values, at OP's question on [Muting / ignoring PHP Notices from a part of my code](https://stackoverflow.com/questions/70906701/muting-ignoring-php-notices-from-a-part-of-my-code) – Markus AO Feb 02 '22 at 20:09