Remove useless call to count()

Nice catch!
With the proposed change, it will still fail both on FALSE and on empty array, so it is good.

I see you don't like my little trick with if (1 which allows to have symmetry of && parts :)
I know it is unorthodox, but it is nice in git, you can add and remove conditions or move them around and only have one line of change (or two for moving).

Do you have a strong opinion about it?

(I would not put this in a patch for a module I don't maintain.)

"Conditions should not be wrapped into multiple lines."

There is already a proposal to change this. #1539712: [policy, no patch] Coding standards for breaking function calls and language constructs across lines

The existing convention is harmful. I have seen so many bugs in contrib modules that would have been spotted much earlier if these super long conditions were broken to multiple lines.

it's another token to read and process.

True.
Ideally, the "compiler" (when filling the opcache) would optimize this away. But I don't think PHP is that smart yet. I saw them discuss compiler optimizations on the internals mailing list.

I see. I guess I should remove it, if it makes it harder for others to read.

I will have a second look at this and actually try it out later, but I think it looks good.

I hope you are having fun with the module!

This patch works in the sense that it addresses the count() issue.

Good points in the discussion above regarding multi-line conditions. Ultimately each developer is going to have their own coding idiosyncrasies which are not inherently wrong. I like the intention behind if (1, but I probably wouldn't have it in my own code primarily because it looks like debugging code, and there is no precedent in Drupal coding styles for it. However, it's used in many other places in the crumbs module, so you may prefer to keep it in for that reason.

It would be great if you could commit this soon because <= PHP 7.1 is already EOL.

Committed to 7.x-2.x.
Thanks for testing!

I kept the if (1, I think the commit is more minimal like this.
Even more minimal with git show 35f28b85 --word-diff.

Hi
I already merged this long ago, but now I am a bit confused.

The intention appears to be to make sure entity_load() didn't return FALSE which is not countable anyway and will lead to a warning in PHP 7.2.

When does entity_load() ever return FALSE?

The count() is not technically needed, but it is not as wrong as is claimed in the issue summary.