Assignment and condition evaluation in one step

Question

Hi I have this code

def errorMap = validateParams(params)
if (errorMap) {
    flash.errorMap = errorMap
    return
}

My question is this: Can I combine the assignment in line #1 and evaluation of the condition in line#2 to make a one liner like the following:

if (flash.errorMap = validateParams(params)) {
   return
}

Is it a bad practice to do this?

Thanks

Vijay Kumar

Answer 1

In my opinion it's a very good practice. Calling the function and testing its return value should be thought of together, and putting them together in the source code helps to do that. If you do this habitually, it becomes essentially impossible to accidentally call the function but leave out the code check whether it succeeded.

Answer 2

If this is C++ or C# code, you can combine assignment and evaluation of condition. Just be absolutely certain to avoid using assignment (=) instead of comparison (==). You can waste hours figuring it out.

Also, be careful about using conditions that modify their parameters.

For example, if (x++ > 100) doStuff()

vs

if (x+1 > 100) doStuff()

Answer 3

We are indoctrinated in C-like languages that single-equals "=" should look like a typo in an if statement. Using a syntax where the single-equals is doing what you mean makes it harder to spot the typo cases.

Although you certainly can do this, my own two cents is that it's usually a bad practice. It's terse, but your if statement is now relying on the evaluation of the assignment, which may not be immediately obvious when you come back and revisit this code months later.

Answer 4

While the code in isolation looks precise and elegant, an assignment operator (=) inside the if-clause is more likely to be overlooked as the more prevalent comparison operator (==), which will then cause you more problems.

I wouldn't use it in practice. It could make a good multiple-choice question though.