Don't use count() to check if array is empty.

Problem

There are a number of cases in features where count() is used to check if an array is empty.

    $ignore_keys = _features_get_ignore_keys($component);
    // remove keys to be ignored
    if (count($ignore_keys)) {

In some cases the result is compared to zero:

return (is_array($array) && (0 !== count(array_diff_key($array, array_keys(array_keys($array)))) || count($array)==0));

In my own benchmarks, count() can be up to 3x slower than alternatives. This can become relevant if we repeat the same operation a lot of times, e.g. in _features_remove_recursion().

Scope

I want to focus here on cases where we expect the value to be an array, and we only want to check if it is empty or not.
E.g. we are not discussing cases where the value could be an iterator.

The behavior on non-arrays is irrelevant, or can be checked in a separate condition.

There are cases where we want to check emptiness, and other cases where we want to check non-emptiness.

In many cases we are already in a context where we get the implicit cast to boolean for free. E.g. in an if() condition, or in a boolean expression with && or ||.

Alternatives

There are simpler/faster ways to check if an array is empty.

1. Implicit or explicit cast to boolean

if ($arr) {
  // Array is not empty.
}
$is_not_empty = (bool) $arr;
$is_empty = !$arr;
if (array_diff_key($arr, array_keys(array_keys($arr)))) {
  // Array is "associative".
}

Good:

• Likely the fastest option in most cases.
• Least verbose.

Bad:

• From looking at the code, it is not immediately obvious that we expect $arr to be an array.

Behavior on non-arrays:

• Simply checks if the value is true-ish.
• Iterators are considered non-empty, even if they are technically empty.

2. Call empty()

if (!empty($arr)) {
  // Array is not empty.
}
if (!empty(array_diff_key($arr, array_keys(array_keys($arr))))) {
  // Array is "associative".
}

Good:

• Almost as fast as the cast to bool above, qualifies as "fast enough".

Bad:

• From looking at the code, it is not immediately obvious that we expect $arr to be an array.
• empty() also checks if the variable is defined at all. This means it will suppress IDE inspection warnings for undefined variables.
• Additional layer of parentheses.

Behavior on non-arrays:
It simply checks if the value is (not) empty-ish.
Mostly the same as with cast to boolean, except for some funny cases in older PHP versions:
https://3v4l.org/LBkjG (enable "EOL versions")

3. Compare to empty array

if (array() !== $arr) {}
if ([] !== $arr) {}
if ($arr !== []) {}
if ([] !== array_intersect_key($arr, array_keys(array_keys($arr)))) {}
if (array_intersect_key($arr, array_keys(array_keys($arr))) !== []) {}
if (array() !== array_intersect_key($arr, array_keys(array_keys($arr)))) {}
if (array_intersect_key($arr, array_keys(array_keys($arr))) !== array()) {}

Good:

• Almost as fast as the above, qualifies as "fast enough".
• The code makes it super obvious that we expect $arr to be an array.
• Beautiful and short with short array syntax and yoda style.

Bad:

• We are not supposed to use yoda conditions and short array syntax in D7 :(

Behavior on non-arrays:

• Non-arrays are considered non-empty.

Iterators are considered non-empty, even if they are technically empty.

4. (bool) count()

if (count($arr)) {}
if (count(array_intersect_key($arr, array_keys(array_keys($arr))))) {}

Good:

• Obvious that we expect $arr to be an array.

Bad:

• Up to 3x as slow as the alternatives (depends what and how you measure, of course).
  Computes an intermediate integer value (the count) that contains more information than what we finally need to determine emptiness.
• Additional layer of parentheses.
• Implicit conversion from integer to boolean.

Behavior on non-arrays:

• Most non-array values return 1 when counted.
• count(null) is 0.
• count($iterator) is 0 if the iterator is empty.
• Since PHP 7.2, count(*) still returns the same result, but produces a warning when called with non-array values.

4. 0 !== count()

if (0 !== count($arr)) {}
if (0 !== count(array_intersect_key($arr, array_keys(array_keys($arr))))) {}
if (count($arr) !== 0) {}
if (count(array_intersect_key($arr, array_keys(array_keys($arr)))) !== 0) {}

Good:

• Obvious that we expect $arr to be an array.
• Explicit "conversion" from integer to boolean (less "magic" than the above).

Bad:

• Up to 3x as slow as the alternatives without count (difference to previous version with implicit cast is insignificant).
  Computes an intermediate integer value (the count) that contains more information than what we finally need to determine emptiness.
• Additional layer of parentheses.
• Additional explicit operation (the ===).

Behavior on non-arrays:
Same as above. count() always returns an integer, and casting that to boolean is equivalent to comparing to zero.

• Most non-array values return 1 when counted.
• count(null) is 0.
• count($iterator) is 0 if the iterator is empty.
• Since PHP 7.2, count(*) still returns the same result, but produces a warning when called with non-array values.

Conclusion

For cases where we expect the value to be an array, count($arr) can be replaced with "array() !== $arr".
- Yoda condition is already used in other places in features, so let's be rebels and go with it.
- Short array syntax is not used anywhere in features, and would feel super inconsistent. So let's stick with "array()" instead of "[]".

Implementation plan

I (donquixote) am doing local clean-up work on features, and will funnel different commits into different branches.
All commits related to count() will go here.

Priority

This change is not super important, it is just a fallout from overall cleanup/refactoring work.

Risk?

All of these refactorings are meant to be 100% equivalent with the old version.
Let's have another look when we have some code.

Some of the functions being modified could be covered with additional unit tests.