Best way to format the conditional checks in "if" statement

Question

This code looks dirty and I can't figure out how to format it so that I can read it, understand it, and look clean at the same time.

if(center==2 && ((((y-height/2)==j) && ((x+width/2)==i)) || (((y+height/2)==j) &&  ((x+width/2)==i))))
  regenerateDot(i+1, j, dots); 

Any suggestions?

Zolly

Answer 1

I would break down the boolean expressions into variables named for readability. Something like:

bool isCentered = center == 2;
bool inLowerRegion = (y-height/2) == j && (x+width/2) == i;
bool inUpperRegion = (y+height/2) == j && (x+width/2) == i;
bool inEitherRegion = inLowerRegion || inUpperRegion;

if (isCentered && inEitherRegion) {
   regenerateDot(i+1, j, dots);
}

Apothecium

+ 1 for beating me to a similar answer – Casto

One small suggestion, you don't need extra braces after you put separate bool for the conditions. For example, bool inLowerRegion = (y-height/2)==j && (x+width/2)==i;. Also, inEitherRegion can easily be replaced with inLowerRegion || inUpperRegion in the original if condition; why to introduce extra variable. – Accad

Woah, that's just sexy. Thanks! – Zolly

@iammilind: Good call on the extra parentheses, I'll take them out. The inEitherRegion extra variable is a matter of taste. To me, it makes the if more readable. – Apothecium

I would definitely consider using functions instead of local variables. They increase the main function length and introduce new variables to track (brain overload). – Shibboleth

I would probably use functions if these calculations were made elsewhere as well. If they were only made here, I'd probably stick with local variables. – Apothecium

Answer 2

Consider refactoring. You could put sub expressions into their own functions, thus naming their purpose.

For example:

if (IsCentered(center) && IsInsideLower(y, j, i) && IsInsideUpper(y, j, i))
  regenerateDot(i + 1, j, dots);

Note that in the above example the function names might be bogus (I haven't really attempted to understand what the purpose of the code is), but you should get the picture.

Anglin

+1 for readability and conciseness. Good function names means I need no trouble myself with looking their description up! – Shibboleth

Answer 3

At most you can remove extra braces, add some spaces and put the logical partitions in different lines as,

if(center == 2 && 
  (((y - height/2) == j || (y + height/2) == j) && (x + width/2) == i))
{
  regenerateDot(i+1, j, dots);
}

Edit: You have one redundant condition (x + width/2) == i which I have optimized here.

Accad

Answer 4

Almost all the parenthesis are redundant... and adding some whitespace it becomes:

    if(center == 2 && 
        (y - height/2 == j && x + width/2 == i || 
         y + height/2 == j && x + width/2 == i))
        regenerateDot(i+1, j, dots);

Cheju

Removing braces completely sometimes creates confusion in reader's mind. IMO, (y - height/2) == j is worth keeping. – Accad

@iammilind: disagree. If someone doesn't know the language, it's her problem. It's like the people who don't use feature X (exceptions, templates, for(;;), RAII, etc etc...) just because it's 'too complicated' for someone. – Cheju

Answer 5

For something complicated I'd probably break it down into what each condition (grouped by shared &&) is trying to signify and assign it to a sensible variable name.

Casto

Answer 6

I would do it like this

if (2 == center &&  
    (((y - height/2) == j && (x + width/2) == i) ||  
     ((y + height/2) == j && (x + width/2) == i))
   )
{ 
  regenerateDot(i + 1, j, dots); 
}

Photobathic

Answer 7

This is the same as the code you posted:

if( center == 2 )
{
    if( (x+width/2) == i )
    {
        if( (y-height/2) == j ) || (y+height/2) == j ) )
        {
            regenerateDot(i+1, j, dots); 
        }
    }
}

Immovable

Answer 8

Re-ordering it would give something like :

if (center==2 && (i-x)==(width/2) && abs(j-y)==(height/2))
    regenerateDot(i+1, j, dots); 

Lorianne

It would be nice if the person that downvotes an answer also gives a reason for doing so. – Lorianne