Styling compound AND / OR if statements

Question

How do you style complex compound AND / OR if statements for maximum readability? How do you indent and where do you place the line breaks? My particular situation is something like the following. It's definitely better than smashing everything into one line, but still looks messy.

if (
    (
        x == y
        && a != b
        && p.isGood() 
        && (
            i + u == b
            || q >= a
        )
    )
    || k.isSomething()
    || m > n
) {
    doSomething();
}

Answer 1

I usually re-factor my code to be more modular if my conditionals get that complicated.

Answer 2

I'd do something more like this, at this level of complexity

bool doIt = x == y && a != b && p.isGood();
doIt &= ( i + u == b || q >= a);
doIt |= k.isSomething() || m > n;

if(doIt)
{
    doSomething();
}

it's ugly, but it's readable and I'm pretty certain the compiler will know how to refactor it.

On the other hand, if I ever see myself in the situation of writing such an IF statement, I rethink the solution, because I'm CERTAIN there's a way of doing it simplier, or at least abstracting some of that condition (e.g.: maybe x == y && a != b && p.isGood() really just mean this->isPolygon() and I can make that method;

Answer 3

Make boolean variables for each small steps:

bool step1 = i + u == b || q >= a;
bool step2 = a != b && p.isGood() && group1;
bool step3 = group2 || k.isSomething() || m > n;
if (step3) { doSomething(); }

This is of course similar to Lacrymology's answer, except with different names for each step.

If you name step1, step2 and step3 in ways that make good conceptual sense, this should be by far the most legible. p.isGood() and k.isSomething() may sometimes be invoked in situations where it wouldn't be in your original code, so this wouldn't be an option if those functions are expensive or if you're running this code in a very tight loop.

On the other hand, you needn't worry about the performance hit that creating new variables might incur; a good compiler will optimize them out.

An example with rectangle collision detection (which you probably wouldn't use due to the aforementioned performance hit):

if((a.x + a.width >= b.x || b.x + b.width >= a.x)
 && (a.y + a.height >= b.y || b.y + b.width >= a.y)
)
{ collision(); }

Might become:

bool horizMatch = a.x + a.width >= b.x || b.x + b.width >= a.x;
bool vertMatch = a.y + a.height >= b.y || b.y + b.width >= a.y;
if(horizMatch && vertMatch) { collision(); }

Also, if you want to leave your code as is, I think that would be totally fine too. I honestly think your code is quite legible. Obviously I don't know what exactly a b x y i u p k m n are, but as far as structure goes, it looks good to me.

Answer 4

I'm getting less obsessed with vertical alignment over time, but my general form with mult-line expressions is...

if (   (   (expr1 == expr2)
        || (expr3 == expr4)
        || (expr5 == expr6)
       )
    && (   (expr7 == expr8)
        || (expr9 == expra)
       )
   )
{
  blah;
}

Key points...

• The close parens align vertically with the open parens, as with the braces.
• The sub-expressions that fit within one line are within one line, and are vertically aligned at the left. Where it helps readability, the infix operators within those single-line parts are vertically aligned too.
• The closing braces naturally create nearly-blank lines, helping to visually group things.

Sometimes, I'll format + and * or some other operators like this too. Quite a few complex expressions take a sum-of-product or product-of-sum form (that can refer to boolean "sums" and "products") so it's probably common enough that a consistent style for it is worthwhile.

Be careful with this, though. It's often better to refactor (move parts of the expression into a function, or calculate and store intermediate parts in a variable) rather than using indentation to try to make an overcomplex expression more readable.

If you prefer to stack your close-parens on the right-hand-side, I don't hate it, but I guess it's not too bad. Taken too far, you run the risk that a mistake can leave the indentation misrepresenting what the parentheses do, though.

if (   (   (expr1 == expr2)
        || (expr3 == expr4)
        || (expr5 == expr6))

    && (   (expr7 == expr8)
        || (expr9 == expra)))
{
  blah;
}

Answer 5

http://www.codinghorror.com/blog/2006/01/flattening-arrow-code.html

I agree with JohnFx's answer as well as one by Lacrymology. I would build a bunch of functions (preferably static) that accomplish small goals and then build up on them in a smart way.

So, how about something like this? Note, this is not the perfect solution, but it works. There are ways to clean this up further, but more specific information is needed. Note: this code should run just as fast, for the compiler is smart.

// Currently based on members or global vars
// (which is often a bad idea too)
function doSomethingCondirionally()
{
  if (k.isSomething() || m > n)
  {
    doSomething();
    return;
  }

  // Else ... 
  if (x != y) return;
  if (a == b) return;
  if (!p.isGood()) return;

  // Final, positive check
  if (i + u == b || q >= a)
  {
    doSomething();
  }
}

Answer 6

For what it's worth, I was surprised to see that your example looks a lot like the complicated predicates I've written. I agree with others that a complicated predicate isn't the greatest for maintainability or readability, but occasionally they come up.

Let me emphasize that you did this part correct: && a != b NEVER put logical connector at the end of a line, it's too easy to miss visually. Another place where you should NEVER put an operator at the end of the line is in string concatenation, in languages with such an operator.

Do this:

String a = b
   + "something"
   + c
   ;

Don't do this:

String a = b +
   "something" +
   c;

Answer 7

If the conditional is that complicated, it is usually an indication that it should be broken up into parts. Perhaps one clause can be assigned to an intermediate variable. Perhaps one clause can be turned into a helper method. I generally prefer not to have so many ands and ors in one line.

Answer 8

You could break up the code into multiple statements, making it easy to understand. But a real ninja would do something like this. :-)

if
(
    (
        x == y
    &&
        a != b
    &&
        p.isGood()
    &&
        (
            i + u == b
        ||
            q >= a
        )
    )
||
    k.isSomething()
||
    m > n
)
{
    doSomething();
}