On empty return values
Posted:
Earlier today, I posted a brief tweet (isn't that redundant?) about return values in PHP (or really, any language). Originally it was about return values from functions (such an exciting topic, I know), but it ended up generating a fair bit of lively conversation, as well as a patch against Drupal 8. So lively, in fact, that I think it deserves more than 140 characters.
New rule:
If your function returns a collection, its null value return must also be a collection.The original question is one of predictability: If you have a function that return an array of things, and there are no things to return, you should return an empty array. Not false, not
NULL, an empty array. Why? Because I want to foreach() over it. Consider the following code snippets:
$items = get_items();
if ($items) {
foreach ($items as $item) {
$item->doWhatever();
}
}
Because get_items() may return NULL or FALSE (doesn't much matter here) if there are no items to return we need to check for a non-array value or the foreach() will complain at us (rightly so) for passing it a non-array value. Now consider what happens if the empty-value return from get_items() is an empty array:
$items = get_items();
foreach ($items as $item) {
$item->doWhatever();
}
Much simpler. And we can simplify it even further in a number of ways:
// Easy.
foreach (get_items() as $item) {
$item->doWhatever();
}
// Slick
array_map(function($item) { $item->doWhatever(); }, get_items());
"But wait!" you cry. "I want to do something different if there's no values, not just skip stuff!"
Sure. And in PHP, an empty array evaluates to boolean false. So if you want an if-else case, you can still have it, just as before:
$items = get_items();
if ($items) {
foreach ($items as $item) {
$item->doWhatever();
}
}
else {
print "Where did all my items go?";
}
"But wait!" you cry. "Not having items is an error condition! array() doesn't trigger an error."
Well first, if you're doing basic error handling then the if-else check above still works exactly the same. Second, if it's truely an error condition you have no business using a magic return value for it. Throw an exception. That's what they're there for.
"But wait!" you are still crying. "I'm returning a single record, not a collection of things, so an empty array makes no sense."
Ah, now we have a valid objection, and the key point of the rule above. PHP's loose typing and super-flexible arrays hide this point, but it is key. Just because something is an array data structure doesn't mean it's a collection. In PHP, we have to distinguish between a collection and a struct ourselves.
Specifically, in PHP something is "a collection" if it is traversable. "Traversable" is PHP speak for, roughly, "I can stick it into a foreach() and the engine won't whine at me." Arrays are traversable, but so are many objects if they implement an Iterator interface of some kind. Arrays are also various other things that objects may or may not be. That is, again, an important distinction.
If your function returns a traversable, it must always return a traversable. If your function returns something that I can foreach(), it must always return something I can foreach. In the majority of cases this means the same object type regardless of whether it contains data or not; collections can be empty and still be collections.
So if you are returning a traversable set of database records, the "no records found" case (which is totally legitimate) should be a traversable but empty set of database records. "Oops, the database broke" is not cause to return NULL; it's cause to throw an exception. Don't break your contract: If you say you will return a traversable, return a traversable.
"But my PHPDoc says it will return NULL if nothing is found, so I'm just following my contract."
If you wrote such a contract, hire a lawyer next time you go to write an API because you suck at writing contracts. Contracts in code should make life predictable for all parties involved. "I may give you an array, or maybe NULL, or maybe FALSE" is not predictable. It's a stupid contract. You get one return type. That's it.
And if your contract says it returns not an array but an object collection (either one of those in SPL or one of your own), an empty collection is still correct. Any mass-operations on that collection should still work, and become no-ops. That is, the following should give no errors:
get_things()->manipulateAll()->save();
If I care that get_things() returned an empty set, I'll tell you. Or rather, I'll ask you using the Countable interface, which you should implement.
"But wait!" you are still crying. "That's for collections. What about normal objects, or array-structs?"
(You're such a cry baby...)
First of all, that may or may not be an error condition. If it's an error condition, you should be throwing an exception so the question is moot. If it's not an error condition, you need to consider what you could return instead. NULL is an option; NULL in PHP is a value that indicates the explicit absence of a value. (Oh, PHP...) If you want to specify the explicit absence of a value, that's NULL. It is not FALSE. FALSE is a boolean. It is a value, specifically the opposite of TRUE. It is not the absence of a value of some other type. If a function can return FALSE, then the only other thing it should be able to return is TRUE (or NULL, in weird cases). If you can return a non-boolean, then you must never return a boolean.
Of course, don't get too attached to that NULL. NULL may indicate the absence of value, but it's also a PITA to deal with. Consider the chain of method calls above. What happens if maniplateAll() returns NULL? That's right, fatal error. Please don't cause fatal errors. There may be appropriate cases for NULL. But consider what you could do instead that would not force me to throw is_null() around my code, because throwing is_null() around my code makes me sad. NULL should be the return value of last resort, because it means nothing. (Literally.)
In short: Write good contracts. Obey your contracts. You get one, and only one, return type. Don't make me do extra work because you wrote a dumb contract.
Comments
Personally, I think chaining can be a good or a bad thing, depending on the context. For something like a query builder, which will probably have a fluent interface, chaining is fine. However, chaining isn't fine if it's used to 'reach into dependancies'. If you find yourself doing $blogPost->getCommentModel()->getRepository()->add($newComment), you should probably refactor so you can do $blogPost->addComment($newComment) instead. (This is known as the 'Law of Demeter', http://en.wikipedia.org/wiki/Law_of_Demeter)
I will change that now to throwing errors and always returning the array (or other iteratable object).
It's an uphill battle to bring those tends of thousands of PHP 4 developers into the modern era, one that, unfortunately, the core PHP community has historically done a poor job of.
That's where it's get really weird as you know that null can be returned you simply check:
if ($returnedValue) { echo "then do something"; }What happens is that negative number in if statement gets treated as a true and then somethings happens... typical a fatal error.
This is great article and there should be more of these.
php > var_dump((array) false, (array) null); array(1) { [0] => bool(false) } array(0) { }null gets converted into empty array, but false contains false as its value.
I now understand why so many non-PHP programmers complain about PHP as a language. Aside from its horrid naming conventions, it pretty much constantly sins against the return type contract.
This post really should be about telling people who code in PHP to use exceptions instead of NULL/FALSE for returning an error.
// Slick. I'm thinkin' that should be// Inscrutable. ; )Just a historical note that, since this post/comment was written, PHP 7 now supports return types on all functions and methods. Everyone should be using them always, for exactly the reasons described here.
Great post, if this was adopted more widely, life would at times be much easier.