CODEWITHSUNDEEP
c++code-formatting
Lightyear Buzz
Lightyear Buzz

Reputation: 796

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

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?

Upvotes: 8

Views: 1895

Answers (8)

Ed Heal
Ed Heal

Reputation: 60017

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); 
}

Upvotes: 1

iammilind
iammilind

Reputation: 70030

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.

Upvotes: 2

cdmckay
cdmckay

Reputation: 32280

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);
}

Upvotes: 17

Sander De Dycker
Sander De Dycker

Reputation: 16243

Re-ordering it would give something like :

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

Upvotes: 1

Exa
Exa

Reputation: 4110

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); 
        }
    }
}

Upvotes: 1

Christian.K
Christian.K

Reputation: 49280

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.

Upvotes: 6

Shaun Wilde
Shaun Wilde

Reputation: 8358

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.

Upvotes: 2

Yakov Galka
Yakov Galka

Reputation: 72519

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);

Upvotes: 2

Related Questions