I have a Function which returns the “# of New Friend Requests” that a User has, and am not sure what to return in case that there is a problem with my query in the Function.
Here is my code…
function getNewFriendRequestCount($dbc, $requestee){
// Build query.
$q1 = "SELECT COUNT(requestor)
FROM friend
WHERE requestee=?
AND requestor_approved=1 AND requestee_approved=0";
// Prepare statement.
$stmt1 = mysqli_prepare($dbc, $q1);
// Bind variable to query.
mysqli_stmt_bind_param($stmt1, 'i', $requestee);
// Execute query.
mysqli_stmt_execute($stmt1);
// Store results.
mysqli_stmt_store_result($stmt1);
// Check # of Records Returned.
if (mysqli_stmt_num_rows($stmt1)!==1){
// Friend-Request Count Found.
// Bind result-set to variables.
mysqli_stmt_bind_result($stmt1, $newFriendRequestsCount);
// Fetch record.
mysqli_stmt_fetch($stmt1);
return $newFriendRequestsCount;
}else{
// Friend-Request Count Not Found.
$resultsCode = 'FUNCTION_FRIEND_REQUEST_COUNT_NOT_FOUND_5005';
// Set Source Page. (New)
$sourcePage = $_SERVER['SCRIPT_NAME'];
// Log Function Error.
logError($dbc, $resultsCode, $sourcePage, $requestee);
// End script.
// exit();
// IS THIS OKAY TO DO, OR SHOULD I DO SOMETHING ELSE??
return '';
}
}//End of getNewFriendRequestCount
Originally, I had this line of code after my IF-THEN-ELSE…
return $newFriendRequestsCount;
However, that caused an “Undefined Variable” error when the ELSE branch fired, so I moved things around, and just went with return ‘’ for lack of a better idea?! :-/
If the user has no friends or something went wrong with the query, just return 0 (zero)?
I assume the function is expected to return an integer, thus the “Count” in the name.
What you return on failure (if anything at all) will depend largely on how you see yourself eventually using the code in “userland” (your PHP script).
Because you use the word “Count” in your function name it suggests you return an integer.
If you are going to go on and treat someone with 1 friend different than someone with 100 friends then returning 0 would seem to be correct, because you are maybe going to iterate through that number.
So is it to be:
if( !getNewFriendRequestCount($dbc, $incoming)){
echo 'sad face';
}else{
echo 'well, you have at least one friend';
}
OR ?
if( getNewFriendRequestCount($dbc, $incoming) === 0 ){
echo 'sad face';
}else{
echo 'well, you have at least one friend';
}
If your function was called hasFriendRequests($dbc, $incoming) then this suggests a binary true/false so you’d perhaps have userland code like this:
if( hasFriendRequests($dbc, $incoming) ){
// Only run code that
// affects those with friends
}
In this case it might be more appropriate to return false; (even though 0, as we know, can be assessed as being “false”)
In answer to your question, think about how you would ideally like to use this function in your scripts, do you want it neat and short? or to be the subject of conditional checks? Then name it appropriately so that it is fairly obvious to anyone reading (which could be you in 3 months time) what the purpose is, and even suggests logically what it is going to return.
EDIT
gaarhh … just spotted this in @logic_earth;'s reply…
You can return false or null or you can define some constant just for this case.
define(“NO_FRIENDS”, “NO_FRIENDS”);
of if you use class use class constant:
const NO_FRIENDS = “NO_FRIENDS”
Then return the constant:
return NO_FRIENDS;
or if using class constant:
return FriendsClass::NO_FRIENDS;
Then from the calling class you can check
if(NO_FRIENDS == getNewFriendRequestCount()){
// logic for no friends request
} else {
// logic for when have friends request
}
Another approach is you can use custom exception for this situation:
class NoFriendsException extends Exception{}
Then from your getNewFriendRequestCount() function
instead of return ‘’ you can
throw new NoFriendsException();
Then from calling methods wrap your call to getNewFriendRequestCount() inside try/catch
try{
$numberOfRequests = getNewFriendRequestCount();
// process number of requests here
} catch(NoFriendsException $e){
// do something, for example log the message to a log or show
// error message to user...
}
Generally, I’d either throw an exception if an error is so serious it should stop the flow of the application, or if there were say just no results from a query or something, I’d return false;
Never return just a string for a status or something like that, as it makes your code very awkward to use. If you return a string such as:
return “function failed”
for example, this means you’d have to analyse the function and search for those exact results if you want to handle the error. If instead you did:
return false;
This would mean in your calling code you can now do:
if ($result = getNewFriendRequestCount($dbc, $requestee)){
//$result now contains whatever the successful function call would return - an array, an object, a string or a number or whatever it is
}else{
//you can do whatever you'd like to do in here if the function returns nothing and has therefore failed
}
Never just use arbitrary strings as a return value from a function call. It’ll just make your code harder to use.
*Edit: When I say I’d return false if there were no results from a query, I mean if that should be considered an error. In this case you’re actually counting records, so I’d agree with the guy above who said it would be appropriate to simply return a count of zero for this.
