4

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.

  • 1
    C aficionados love this: while ((c = getchar()) != EOF) – mouviciel Nov 26 '13 at 9:01
  • "if ($a > $b) { ....etc" is already using a function. – Pieter B Feb 28 '17 at 15:28
18

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 = $res\n";
    // do stuff
}
  • i forgot to add that to my question, you may assume return value is not needed after the condition is evaluated. – guenis Nov 26 '13 at 3:21
  • 2
    @guenis: You can assume that, but it may not continue to be true as the code is modified. In my example, it's used in debugging code. – Keith Thompson Nov 26 '13 at 3:23
  • Keith is right, and in the general case functions often have (or later develop) side effects. Your function name strikes me as a little idealised, the "use a variable" convention is often the result of maintenance programming experience. Especially in PHP, where treating variables as booleans works in slightly unintuitive ways. Note the comment here: php.net/manual/en/language.types.boolean.php Being able to inspect the variable can be very handy when someone makes that function return a string instead of a boolean. – Móż Nov 26 '13 at 3:27
8

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.

1

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.

0

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.

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.