Function return values and global errors

Best design practices

[trillianjedi]

Hi all,

I'm in the process of re-designing one of my own sites to use a core API which really just consists of a set of PHP functions that various parts of the site can call.

I'm thinking about error reporting and checking the return value.

If a function returns an integer (which may be signed) what's the best way to deal with errors? I've usually tried to design functions to return true or false depending on success, but what I really need to do is return a success value and a value value.

Example:-

function giveMeAnInteger() {
return "-2000";
}

So in this case I can't do a conditional on the return value like this:-

if (giveMeAnInteger()) {
doSomething;
}

I could supply a vessel for the integer:-

function giveMeAnInteger(&$inhere) {
return ($inhere = "-2000");
}

.... but to me that looks really ugly. Also, how would I return a human readable error message that I could display in the browser?

What I really want to be able to do is something like this:-

function giveMeAnInteger() {
$result = NULL;
if ($result = "-2000") {
$globalError = "Failed to get an int";
}

return $result;
}

Then I should be able to check for NULL, and get the result as a return if it isn't so.

Make sense? I'm really fishing for good design practice here more than anything....

[cameraman]

I return false on error, and always check for it explicitly:
if(($id = getID($arg)) === false) {
} // handle error

You could also implement an error class - since you're thinking site-wide, it would be an excellent time to do it. Define yourself some codes & messages, then any function that encountered/generated an error could set the code. The calling script would be responsible for checking the error class for error(s) and sending the message to the browser.

[trillianjedi]

Thanks Cameraman.

In your function example, getID would return "False" explicitly, and the "===" checks for both type as well as value, right?

You could also implement an error class

How would that be defined?

Thanks...

[pinterface]

Working under the assumption that switching to a language which supports returning multiple values is out of the question, and that what you want isn't

function maybeGetInt ($x) { return $x ? array(7, true) : array(null, false); }
list ($val, $ok) = maybeGetInt($x);
if ($ok) { /* use $val */ }

, and that in some cases you'll need to be able to return any number of types (e.g., both integers and booleans) so cameraman's solution won't work, what's wrong with either Exceptions [us3.php.net] or trigger_error [us3.php.net] and set_error_handler [us3.php.net]?

One of the nice things about exceptions is you can do something like:

try {
 print (7 + maybeGetInteger(...) + maybeGetInteger(...));
} catch (NoIntForYou $e) {
 print "Sorry, I can only add numbers.";
}

rather than

if ($i = maybeGetInteger(...) !== false &&
    $j = maybeGetInteger(...) !== false) {
 print (7 + $i + $j);
} else {
 print "Sorry, I can only add numbers.";
}

Hrm...well, it's hard to show significant improvement in a small example, but it's really quite nice to not have to check return values all the time.

Of course, a more general condition system with restarts would be preferable (e.g., Common Lisp's), but you take what you can get! :)

You've got quite a few options, so the most important thing is to clearly document your function's contract. You'll feel pretty silly if some bug ends up being a result of

"hamburger" + 12

. ;)

[cameraman]

Yes, === checks type as well as value.
Yes, getID() might look something like (pseudo-code):
function getID($arg) {
$qry = mysql_query("SELECT id FROM table where name='$arg'");
if($qry) {
if(mysql_num_rows($qry)) {
$id = mysql_fetch_row($qry);
return $id[0];
} // if have data
else return false;
} // if no error in query
else return false;
} // end getID()

An error class (php5 style) might look something like
class appError {
protected $errs;
protected $msgs;
public $error;
public function __construct() {
$this->error = false;
$this->errs = array();
$this->msgs = array(
11 => "Invalid query",
12 => "Empty record set",
13 => "mySQL error",
21 => "File not found",
31 => "You must be signed in to access this function"
);
}
public function setErr($codeOrMsg,$file,$line) {
if(is_numeric($codeOrMsg))
$this->errs[] = array(
'code' => $codeOrMsg,
'file' => $file,
'line' => $line
);
else
$this->errs[] = array(
'code' => 999,
'msg' => $codeOrMsg,
'file' => $file,
'line' => $line
);

$this->error = true;
} // end setErr()
public function getLast() {
if(!count($this->errs)) return false;
$last = array_pop($this->errs);
if(!count($this->errs)) $this->error = false;
if(isset($last['msg']))
$rtn = $last['msg'];
else
$rtn = $this->msgs[$last['code']];
$rtn .= " in file " . $last['file'];
$rtn .= " on line " . $last['line'];
return $rtn;
} // end getLast()
public function getAll() {}
public function clear() {}
} // End appError class

at top of a script:
$oops = new appError();

in a function (or body of script, doesn't matter), for example:
function getFile($filename) {
global $oops;
if(!file_exists($filename)) {
$oops->setErr(21,__FILE__,__LINE__);
return(false);
}
else return file_get_contents($filename);
}

$billybob = getFile("billybob.txt");
if($oops->error)
echo $oops->getLast();
else
echo $billybob;

pinterface raises a good point, you obviously can't return false to indicate an error if false is a valid return value. I don't find it at all inconvenient to work with arrayed return values, but then again 98% of what I do involves a database somewhere so I'm getting record data back and it's generally several fields. I don't use list() in those circumstances; pinterface is right, the way that code snippet is written it's about as messy as you can make it.

Another good point, ideally it's better to validate input before you begin to manipulate it, otherwise you've wasted processor time and things can get really messy like that example.

Personally I don't much care to surround code statements with try{} catch{} blocks because I think it's messy and harder to maintain, but that's just a personal preference; everyone has them. If you're going to be robust about catching and recovering from errors there's going to be some lengthy code somewhere, so you have to figure out what's easiest for you to deal with.

[trillianjedi]

Thanks guys.

set_error_handler?

That looks promising. If I can use an existing functionality of PHP, all the better.

Thanks for the examples camerman - that's very helpful. I'm not averse to using array returns at all. That may be the most appropriate thing to do.

In general, how about returning NULL if there's an error, and setting the global $err variable using et_error_handler, in the event that something didn't work?

That would seem to me to be the best of both worlds? I would never use NULL as a valid return value for a function. Returns would either be a value unless something went wrong.