Do you think it is a good practice to use function return values as if conditions? I’m coding in PHP atm but it holds for many other languages.
if(isTheConditionMet($maybeSomeParams))
{
}
or
$res = isTheConditionMet($maybeSomeParams);
if($res)
{
}
I can’t think of a situation where the first one creates a problem, can it?
EDIT: Assume return value is not going to be used after condition is evaluated.
2
In the code you posted, there’s no reason to store the function result in a variable before using it. (Unless perhaps the name of the variable is sufficiently meaningful that it makes the code clearer to the reader.)
On the other hand, if you need to refer to the value more than once, you should probably store it in a variable rather than calling the function multiple times, which could be inefficient and/or wrong:
$res = isTheConditionMet($maybeSomeParams);
if ($res)
{
echo "res = $resn";
// do stuff
}
3
The only case where the former can cause a problem, or be annoying, is when you use a debugger that doesn’t or can’t evaluate the function while you’re debugging. Until recently, this was the case for .NET functions/visual studio.
I can’t say for certain, but I would suspect that a decent optimizer would optimize the variable out to conserve memory.
Here’s another situation that you really need to think about:
if(isTheFirstConditionMet($maybeSomeParams) and isTheSecondConditionMet($maybeSomeParams))
{
}
or
$res = isTheFirstConditionMet($maybeSomeParams);
$res2 = isTheSecondConditionMet($maybeSomeParams);
if($res and $res2)
{
}
Now suddenly the code doesn’t behave the same anymore. Because in the first case the second condition will only be evaluated if the first condition is true. But when you assign to temp variables in the second case, both functions will always be evaluated.
The case where you are using multiple conditions you really need to think about behavior you want.
Php and most other languages I know use short circuit evaluation, which means they will do the minimal number of evaluations to get to the result.
If you can look at it the next day and it’s obvious and easy to read, there’s no need for the assignment. But it had a ton of args, or was accompanied by many other function calls in those if parens along with a bunch of operators, I’d drop it in a well-named var to explain what the crap you’re actually intending to switch on.
Assuming that the code is working fine, and you do not need to debug it.
Since, you are not going to use the variable $res, you shouldn’t have it there either. In code block 1 there is one comparison operation on the return value of isTheConditionMet
.
In code block 2 the return value is being assigned to $res and then being compared in if. Why have an extra assignment operation.