API page: http://api.drupal.org/api/drupal/modules--taxonomy--taxonomy.module/func...
Describe the problem you have found:
I ask myself always what gets returned, when no conditions are matching? Its not clear, if that will be NULL and empty array or and Exception. I clicked myself through the stack of calls, but always is only the positive result documented, that an array of Objects with the ids as keys is returned.
Please add always also the result, when no conditions are matching.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | non-matching_return_value_doc-1161240.patch | 1.55 KB | jhodgdon |
| #8 | non-matching_return_value_doc-1161240.patch | 1.52 KB | barbi |
| #5 | non-matching_return_value_doc-1161240.patch | 1.55 KB | barbi |
| #3 | non-matching_return_value-1161240.patch | 1.23 KB | barbi |
Comments
Comment #1
jhodgdonThanks for reporting this!
It looks like we need to document what the return value is when no conditions match for:
taxonomy_term_load_multiple
entity_load_multiple
DrupalEntityControllerInterface::load
Looking at the actual implementation of the entity controller interface here:
http://api.drupal.org/api/drupal/includes--entity.inc/function/DrupalDef...
It looks like it probably returns an empty array if there are no results to the query, since it just calls fetchAllAssoc() on the executed query. So that should be added to the documentation of these three functions/methods.
This is probably a good project for a novice doc contributor...
Comment #2
jhodgdonOh, and it needs to be done in d8 first, then backported to d7.
Comment #3
barbi commentedUsing String "When no results found, an empty array is returned."
If you have a better suggestion, please comment.
Comment #4
jhodgdonI think this is probably fine, but it should be verified. Someone posted an issue recently saying that actually an empty query returns NULL when run through fetchAllAssoc().
Also, it needs an "are" in there:
When no results are found, an empty array is returned.
And this sentence should be started on the previous line in all cases, and wrapped to as close to 80 characters per line as possible without going over.
Comment #5
barbi commentedAdded "are" and concatenated with the earlier line.
Todo : verify return value. (First line, above comment)
Comment #6
jhodgdonText is fine. As noted above, leaving at "needs review" until someone verifies that a no-results query gets an empty array and not NULL as a result.
Comment #7
jhodgdonSee http://drupal.org/node/1167218#comment-4519188
Apparently if there are no results, fetchAssoc() returns boolean FALSE. So this patch needs to be changed accordingly.
Comment #8
barbi commentedComment #9
jhodgdonMy bad... I was confused between fetchAssoc() and fetchAllAssoc(). the non-All function returns FALSE if empty, and the All function returns an empty array. See:
http://drupal.org/node/1167218#comment-4525016
So we need to go back to the patch in #5, which is correct.
And incidentally, regarding the patch in #8, we always use TRUE/FALSE not true/false, according to Drupal coding standards, for Boolean values, for future reference.
Anyway, I am re-attaching the patch in #5 to avoid confusion about which patch we want. Please commit to d7/8! :)
Comment #10
webchickCommitted to 8.x and 7.x. Thanks!