In #phpc we recently had a discussion about function return values; specifically from database queries.
I’m going to go on a (admittedly, rather sturdy looking) limb and say this applies to pretty much any function that returns from a data resource, not just a database .
My preference, is to return false only for error conditions, or when a function is supposed to only return one result (i.e. when selecting on a primary key) and fails to find any result.
However, it’s very rare that I care about whether I hit an error condition or just got no result when it comes to display — more to the point, my user doesn’t care.
There is a simply solution though, an empty array, also evaluates as false.
[php]
function getData()
{
if (error()) {
return false;
} elseif (noData()) {
return array();
} else {
return myData();
}
}
if (!$data = getData()) {
foreach ($data as $row) {
// Show data
}
} else {
echo “No data found.”;
}
// or
$data = getData();
if ($data === false) {
myLog(“Something Bad Happened!”);
}
if (!$data) {
echo “No data found.”;
} else {
// iterate
}
[/php]
This gives the best of both worlds IMO — the ability to distinguish error conditions or not depending on if you need to in the given context.
– Davey
P.S.
This might be the start of a series of thoughts on common API best practices
Comments
Vid Luther
Just curious why you would return an empty array which equates to a false, but not a null for a function named getData? I find it more intuitive when a function named isAvailable, or something that returns true or false, but on error conditions I get a NULL. If there was no error, but the query returned 0 rows, then an empty array may make sense.
Dunno, maybe I need to sleep :)
Joshua May
Well, I missed the conversation, but I see no reason why exceptions aren’t being used more for, y’know, the exceptional cases.
If it’s an error, raise an exception. (and of course, if it’s empty, then it’s not an error, and a plain array() is fine..)
The caller then has the same control, and IMO it removes the ambiguity of return types.
Nicu
I agree with what you are saying and I think some people already use this kind of return values by using code similar to this:
function selectRows($query) {
$result = array(); // this will be returned if no data is found (an empty array)
$r = mysql_query($query);
if (!$r) {
return false; // there was some error
}
while (($row = mysql_fetch_assoc($r)) != null) {
$result[] = $row;
}
return $result; // retrun data (empty or not)
}
This means you don’t have to specifically test if there is no data, you just return the result.
At least this is how my code looks like.
Stefan Priebsch
Exceptions to the rescue! One of the advantages of using exceptions is the clear distinction between “good” results and errors.
Alexey Zakhlestin
I believe, that the “best practice” would be to throw Exception on error, and return empty array for no-data
Peter
http://de3.php.net/manual/en/language.exceptions.php
’nuff said.
Dagfinn Reiersø
The alternative to returning false is to throw an exception. That gives you more flexibility in where you handle the error, since it can be handled at a higher level if you wish.
Larry Kagan
Most of my existing DB models follow a similar method of returning data except that boolean false is returned only if an error occurred. An empty array or string is returned if no data is found. Thus $data === ” allows me to tell the user no records found while $data === false tells me to trigger a text message to my phone or log entry because of an error and politely tells the user that an error occurred and he should try again in a few minutes.
However, as of the past year or two, any error conditions throw exceptions. So I can avoid the awkward ‘@return mixed Array on success, false otherwise’ phpDoc comment.
Just my 2 cents.
cblin
IMHO, non sense.
The best way to deal with an UNEXPECTED error is an exception.
In your example, you say to the user “No data found” whereas you mean something like “the datasource is not availalble” and THIS make a big difference.
Just my 2 cents,
Loïc Hoguin
Better practice would be to throw an exception on error. Code becomes much more readable and less error-prone if you don’t have to check for the return value for every single function calls. Instead you catch what you need to catch and ignore the rest. Uncatched exceptions can then be handled by a custom error handler. Returning an error value/code/message is doing you no good.
If I remember correctly, the book “Code Complete” explains thoroughly this practice and its benefits.
Brandon Savage
Typically I only return false on error conditions as well. One thing that I also try and do is when an error condition is encountered, it raises a warning or a notice (depending on the severity) so I can see when my functions are failing and why.I only do this when something is *not* supposed to fail though. I don’t do this when an error condition might have happened in the course of running the program (like bad data being entered); only when something should never happen (like the database server failing).
Matthew Weier O'Phinney
Why an empty array? That makes the assumption that you’re returning an array as the result. I would expect a null value makes semantic sense when you simply get no results — it’s the absence of a result, basically.
Ilia Jerebtsov
This is why we have null and exceptions.
function getData(){
if (error()) {
throw new Exception('Error message');
}
// only if the result is a single item and not a list
// if it's a list, myData() should already be an empty array.
if (noData()) {
return null;
}
return myData();
}
try {
if ($data = getData()) {
foreach ($data as $row) {
// Show data
}
}
else {
echo "No data found.";
}
}
catch (Exception $ex) {
myLog("Something Bad Happened!");
}
Cleaner, no shoehorning of magic values, and you can actually report more than one error condition.
OnyxRaven
Best API practice for a database error? an exception.
Rob
Definitely an Exception for an unexpected error…
till
Davey – great idea for a serious, please keep it coming!
Though, if you are trying to establish a best practice guide, then e.g. teach people the fundamentals of return too (e.g. that it allows you to exit early and allows you to avoid if/elseif/else structures), because people will just take your example code, copy it and not apply brain. :-)
By the way, in general I’d also suggest to raise an exception. But people often forget that try/catch can be expensive too. IMHO, it always depends on the type of error. I often dis-regard exceptions if they can happen and the code they come from is not crucial to operating.
Comments are closed.