Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
There may be reasons why you don't do this but usually an entity load function returns FALSE if no entity is returned by entity_load(). For example node_load() does:
return $node ? reset($node) : FALSE;
Patch to follow.
Comment | File | Size | Author |
---|---|---|---|
#13 | fieldable_panels_panes-n1985894-13.patch | 15.45 KB | DamienMcKenna |
Comments
Comment #1
tancComment #2
Dave ReidI'm ok with this cleanup, but this actually only needs to be return reset($entities). The docs for reset() show that it will return FALSE if the array is empty.
Comment #3
merlinofchaos CreditAttribution: merlinofchaos commentedThough the construct is safer if, for some reason, the prior load function returns false or null or an empty array.
Comment #4
Dave ReidI don't think returning anything but an array is even possible from entity_load(). It's either an array or an exception.
Comment #5
tancHere's a patch with the method Dave mentioned.
Comment #6
Dave ReidIt looks like we may need to test some of the Views handlers to make sure they work ok if the function now returns FALSE:
Comment #7
DamienMcKennaPer the documentation for entity_load():
That said, per the issue summary, node_load() does do it slightly differently:
On the other hand, taxonomy_term_load() does this:
.. and taxonomy_vocabulary_load() does this:
.. and user_load() does this:
So, the question is, is there a particular reason taxonomy_vocabulary_load() and user_load() do it one way while node_load() and taxonomy_term_load() will return FALSE if nothing is found?
Comment #8
DamienMcKennaLets fix this for the next release.
Comment #9
DamienMcKennaThis builds upon @tanc's patch. I went through and ensured all occasions where foreach() loops were used or the results of the entity load functions were used, were properly checked to not be empty() first.
Comment #10
mglamanCurious why we need to check empty() if it's returning
return $entities ? reset($entities) : FALSE;
. Couldn't we just keep!$entities
or at least$entities === false
?Maybe it's just bad PHP habits, but the FALSE check and variable definition could be within the if(), that's how Commerce does it in a few places.
Just my two bits reviewing through Dreditor and not applying patch yet.
Comment #11
DamienMcKennaI prefer being verbose about it. A slight update.
Comment #12
jibran+1 to this.
Comment #13
DamienMcKennaRerolled.
Comment #14
DamienMcKennaI double-checked the Commerce module and all of the bundled entities follow the same pattern, so I think this will be fine as-is.
Comment #16
DamienMcKennaCommitted.