How to split up complex conditions and keep short circuit evaluation?
<p>Sometimes conditions can become quite complex, so for readability I usually split them up and give each component a meaningful name. This defeats short-circuit evaluation however, which can pose a problem. I came up with a wrapper approach, but it is way too verbose in my opinion. </p>

<p>Can anybody come up with a neat solution for this?</p>

<p>See code below for examples of what I mean:</p>

<pre><code>public class BooleanEvaluator {

    // problem: complex boolean expression, hard to read
    public static void main1(String[] args) {

     if (args != null &amp;&amp; args.length == 2 &amp;&amp; !args[0].equals(args[1])) {
      System.out.println("Args are ok");
     }
    }

    // solution: simplified by splitting up and using meaningful names
    // problem: no short circuit evaluation
    public static void main2(String[] args) {

     boolean argsNotNull = args != null;
     boolean argsLengthOk = args.length == 2;
     boolean argsAreNotEqual = !args[0].equals(args[1]);

     if (argsNotNull &amp;&amp; argsLengthOk &amp;&amp; argsAreNotEqual) {
      System.out.println("Args are ok");
     }
    }

    // solution: wrappers to delay the evaluation 
    // problem: verbose
    public static void main3(final String[] args) {

     abstract class BooleanExpression {
      abstract boolean eval();
     }

     BooleanExpression argsNotNull = new BooleanExpression() {
      boolean eval() {
       return args != null;
      }
     };

     BooleanExpression argsLengthIsOk = new BooleanExpression() {
      boolean eval() {
       return args.length == 2;
      }
     };

     BooleanExpression argsAreNotEqual = new BooleanExpression() {
      boolean eval() {
       return !args[0].equals(args[1]);
      }
     };

     if (argsNotNull.eval() &amp;&amp; argsLengthIsOk.eval() &amp;&amp; argsAreNotEqual.eval()) {
      System.out.println("Args are ok");
     }
    }
}
</code></pre>

<p><strong>Response to answers:</strong></p>

<p>Thanks for all your ideas! The following alternatives were submitted so far:</p>

<ul>
<li>Break lines and add comments</li>
<li>Leave as is</li>
<li>Extract methods</li>
<li>Early returns</li>
<li>Nested / Split up if's</li>
</ul>

<p><strong>Break lines and add comments:</strong></p>

<p>Just adding linebreaks within a condition gets undone by the code formatter in Eclipse (ctrl+shift+f). Inline comments helps with this, but leaves little space on each line and can result in ugly wrapping. In simple cases this might be enough however.</p>

<p><strong>Leave as is:</strong></p>

<p>The example condition I gave is quite simplistic, so you might not need to address readability issues in this case. I was thinking of situations where the condition is much more complex, for example:</p>

<pre><code>private boolean targetFound(String target, List&lt;String&gt; items,
        int position, int min, int max) {

    return ((position &gt;= min &amp;&amp; position &lt; max &amp;&amp; ((position % 2 == 0 &amp;&amp; items
            .get(position).equals(target)) || (position % 2 == 1)
            &amp;&amp; position &gt; min &amp;&amp; items.get(position - 1).equals(target)))
            || (position &lt; min &amp;&amp; items.get(0).equals(target)) || (position &gt;= max &amp;&amp; items
            .get(items.size() - 1).equals(target)));
}
</code></pre>

<p>I would not recommend leaving this as it is.    </p>

<p><strong>Extract methods:</strong></p>

<p>I considered extracting methods, as was suggested in several answers. The disadvantage of that is that these methods typically have a very low granularity and may not be very meaningful by themselves, so it can clutter your class, for example:</p>

<pre><code>private static boolean lengthOK(String[] args) {
    return args.length == 2;
}
</code></pre>

<p>This would not really deserve to be a separate method at class level. Also you have to pass all the relevant arguments to each method. If you create a separate class purely for evaluating a very complex condition then this might be an ok solution IMO.</p>

<p>What I tried to achieve with the BooleanExpression approach is that the logic remains local. Notice that even the declaration of BooleanExpression is local (I don't think I've ever come across a use-case for a local class declaration before!).</p>

<p><strong>Early returns:</strong></p>

<p>The early returns solution seems adequate, even though I don't favor the idiom. An alternative notation:</p>

<pre><code>public static boolean areArgsOk(String[] args) {

    check_args: {
        if (args == null) {
            break check_args;
        }
        if (args.length != 2) {
            break check_args;
        }
        if (args[0].equals(args[1])) {
            break check_args;
        }
        return true;
    }
    return false;
}
</code></pre>

<p>I realize most people hate labels and breaks, and this style might be too uncommon to be considered readable.</p>

<p><strong>Nested/split up if's:</strong></p>

<p>It allows the introduction of meaningful names in combination with optimized evaluation. A drawback is the complex tree of conditional statements that can ensue</p>

<p><strong>Showdown</strong></p>

<p>So to see which approach I utlimately favor, I applied several of the suggested solutions to the complex targetFound example presented above. Here are my results:</p>

<pre><code>// nested / split if's, with meaningful names
// very verbose, meaningful names don't really help the readability here 
private boolean targetFound1(String target, List&lt;String&gt; items,
        int position, int min, int max) {

    boolean result;
    boolean inWindow = position &gt;= min &amp;&amp; position &lt; max;
    if (inWindow) {

        boolean foundInEvenPosition = position % 2 == 0
                &amp;&amp; items.get(position).equals(target);
        if (foundInEvenPosition) {
            result = true;
        } else {
            boolean foundInOddPosition = (position % 2 == 1)
                    &amp;&amp; position &gt; min
                    &amp;&amp; items.get(position - 1).equals(target);
            result = foundInOddPosition;
        }
    } else {
        boolean beforeWindow = position &lt; min;
        if (beforeWindow) {

            boolean matchesFirstItem = items.get(0).equals(target);
            result = matchesFirstItem;
        } else {

            boolean afterWindow = position &gt;= max;
            if (afterWindow) {

                boolean matchesLastItem = items.get(items.size() - 1)
                        .equals(target);
                result = matchesLastItem;
            } else {
                result = false;
            }
        }
    }
    return result;
}

// nested / split if's, with comments
// less verbose, but still hard to read and easy to create bugs
private boolean targetFound2(String target, List&lt;String&gt; items,
        int position, int min, int max) {

    boolean result;
    if ((position &gt;= min &amp;&amp; position &lt; max)) { // in window

        if ((position % 2 == 0 &amp;&amp; items.get(position).equals(target))) {
            // even position
            result = true;
        } else { // odd position
            result = ((position % 2 == 1) &amp;&amp; position &gt; min &amp;&amp; items.get(
                    position - 1).equals(target));
        }
    } else if ((position &lt; min)) { // before window
        result = items.get(0).equals(target);
    } else if ((position &gt;= max)) { // after window
        result = items.get(items.size() - 1).equals(target);
    } else {
        result = false;
    }
    return result;
}

// early returns
// even more compact, but the conditional tree remains just as complex
private boolean targetFound3(String target, List&lt;String&gt; items,
        int position, int min, int max) {

    if ((position &gt;= min &amp;&amp; position &lt; max)) { // in window

        if ((position % 2 == 0 &amp;&amp; items.get(position).equals(target))) {
            return true; // even position
        } else {
            return (position % 2 == 1) &amp;&amp; position &gt; min &amp;&amp; items.get(
                    position - 1).equals(target); // odd position
        }
    } else if ((position &lt; min)) { // before window
        return items.get(0).equals(target);
    } else if ((position &gt;= max)) { // after window
        return items.get(items.size() - 1).equals(target);
    } else {
        return false;
    }
}

// extracted methods
// results in nonsensical methods in your class
// the parameter passing is annoying
private boolean targetFound4(String target, List&lt;String&gt; items,
        int position, int min, int max) {

    return (foundInWindow(target, items, position, min, max)
            || foundBefore(target, items, position, min) || foundAfter(
            target, items, position, max));
}

private boolean foundAfter(String target, List&lt;String&gt; items, int position,
        int max) {
    return (position &gt;= max &amp;&amp; items.get(items.size() - 1).equals(target));
}

private boolean foundBefore(String target, List&lt;String&gt; items,
        int position, int min) {
    return (position &lt; min &amp;&amp; items.get(0).equals(target));
}

private boolean foundInWindow(String target, List&lt;String&gt; items,
        int position, int min, int max) {
    return (position &gt;= min &amp;&amp; position &lt; max &amp;&amp; ((position % 2 == 0 &amp;&amp; items
            .get(position).equals(target)) || (position % 2 == 1)
            &amp;&amp; position &gt; min &amp;&amp; items.get(position - 1).equals(target)));
}

// BooleanExpression wrappers revisited
// note that the method parameters must be declared final
// for this complex case the verbosity is defendable IMO
// Maybe closures will make this easier, if they ever agree on that (-;
private boolean targetFound5(final String target, final List&lt;String&gt; items,
        final int position, final int min, final int max) {

    abstract class BooleanExpression {
        abstract boolean eval();
    }

    BooleanExpression foundInWindow = new BooleanExpression() {

        boolean eval() {
            return position &gt;= min &amp;&amp; position &lt; max
                    &amp;&amp; (foundAtEvenPosition() || foundAtOddPosition());
        }

        private boolean foundAtEvenPosition() {
            return position % 2 == 0 &amp;&amp; items.get(position).equals(target);
        }

        private boolean foundAtOddPosition() {
            return position % 2 == 1 &amp;&amp; position &gt; min
                    &amp;&amp; items.get(position - 1).equals(target);
        }
    };

    BooleanExpression foundBefore = new BooleanExpression() {
        boolean eval() {
            return position &lt; min &amp;&amp; items.get(0).equals(target);
        }
    };

    BooleanExpression foundAfter = new BooleanExpression() {
        boolean eval() {
            return position &gt;= max
                    &amp;&amp; items.get(items.size() - 1).equals(target);
        }
    };

    return foundInWindow.eval() || foundBefore.eval() || foundAfter.eval();
}
</code></pre>

<p>I guess it really depends on the situation (as always). For very complex conditions the wrapper approach might be defendable, although it is uncommon. </p>

<p>Thanks for all your input!</p>
