Short conditions in IFs


Link to this posting

Postby Ursego » 21 Feb 2013, 21:56

Don't write complex logical expressions in if statements. Instead, evaluate the expression in advance, store its result in a Boolean variable with a quality, descriptive name, and use that variable in the if statement. If the evaluation of a logical expression is longer than a few lines of code, then refactor it to a brand new Boolean function, and call it from within the if statement.

That will make the logic being implemented extremely easy to understand because the code will be very self-explaining.

*** BAD code: ***

Code: Select all
if ((calcMethod == CalcMethods.ADDITIVE && additiveCalculationPassed) || (calcMethod == CalcMethods.RATIO && ratioCalculationPassed))...

Wow, that's hard to understand! But if you use a Boolean variable (or a function) to hide the expression's complexity, the result is code that is virtually as readable as "straight" human language. The previous (bad) code says only WHEN the condition is true, but the good code tells us both WHEN and WHY:

*** GOOD code: ***

Code: Select all
boolean structuralChangeOccurred;

structuralChangeOccurred =
      (calcMethod == CalcMethods.ADDITIVE && additiveCalculationPassed) ||
      (calcMethod == CalcMethods.RATIO && ratioCalculationPassed);
       
if (structuralChangeOccurred)...

Now, it is much easier for anyone to read your code; you can literally read it. If you then need to understand how the Boolean variable is computed, you can look "under the covers".

The expression in my example is very primitive because it's enough to convey the idea. In the real life, I have seen statements with half-a-screen Boolean expressions inside an if!

Break a complex logical expression into simple expressions and populate the Boolean variable step by step rather than in one assignment statement.

Come to the truth in a few small steps. Use a number of simple Boolean expressions rather than one monstrous construction:

*** BAD code: ***

Code: Select all
boolean theShowMustGoOn;

theShowMustGoOn = ([cond1] || [cond2]) || ([cond3] && [cond4] && this.isXxx()) || ([cond3] && this.isYyy());

if (theShowMustGoOn)...

*** GOOD code: ***

Code: Select all
boolean theShowMustGoOn;

if ([cond1] || [cond2])
   theShowMustGoOn = true;
elseif ([cond3] && [cond4])
   theShowMustGoOn = this.isXxx();
elseif ([cond3])
   theShowMustGoOn = this.isYyy();
else
   theShowMustGoOn = false;

if (theShowMustGoOn)...

It's not only more readable. It also:

* Simplifies debugging. In the BAD code, it's much harder to investigate which of the many conditions caused the variable to be populated. If you want to STEP IN a function, you will be forced to STEP IN all the functions, which appear in the expression before.

* Potentially, improves performance. The functions (which can be not very efficient if they call a web service or contain a heavy database query) have a chance not to be called at all (if a previous validation has already produced a Boolean result).
User avatar
Ursego
Site Admin
 
Posts: 131
Joined: 19 Feb 2013, 20:33



IF you want to ((lose weight) OR (have unbelievable (brain function AND mental clarity))) THEN click:




cron
free counters

eXTReMe Tracker