javarefactoringif-statementdecomposition

How can I refactor a large block of if statements in Java?


I recently profiled some code using JVisualVM, and found that one particular method was taking up a lot of execution time, both from being called often and from having a slow execution time. The method is made up of a large block of if statements, like so: (in the actual method there are about 30 of these)

    EcState c = candidate;

    if (waypoints.size() > 0)
    {
        EcState state = defaultDestination();
        for (EcState s : waypoints)
        {
            state.union(s);
        }
        state.union(this);
        return state.isSatisfied(candidate);
    }

    if (c.var1 < var1)
        return false;
    if (c.var2 < var2)
        return false;
    if (c.var3 < var3)
        return false;
    if (c.var4 < var4)
        return false;
    if ((!c.var5) & var5)
        return false;
    if ((!c.var6) & var6)
        return false;
    if ((!c.var7) & var7)
        return false;
    if ((!c.var8) & var8)
        return false;
    if ((!c.var9) & var9)
        return false;

    return true;

Is there a better way to write these if statements, or should I look elsewhere to improve efficiency?

EDIT: The program uses evolutionary science to develop paths to a given outcome. Specifically, build orders for Starcraft II. This method checks to see if a particular evolution satisfies the conditions of the given outcome.


Solution

  • First, you are using & instead of &&, so you're not taking advantage of short circuit evaluation. That is, the & operator is going to require that both conditions of both sides of the & be evaluated. If you are genuinely doing a bitwise AND operation, then this wouldn't apply, but if not, see below.

    Assuming you return true if the conditions aren't met, you could rewrite it like this (I changed & to &&).

    return 
           !(c.var1 < var1 ||
           c.var2 < var2 ||
           c.var3 < var3 ||
           c.var4 < var4 ||
           ((!c.var5) && var5) ||
           ((!c.var6) && var6) ||
           ((!c.var7) && var7) ||
           ((!c.var8) && var8) ||
           ((!c.var9) && var9));
    

    Secondly, you want to try to move the conditions that will most likely be true to the top of the expression chain, that way, it saves evaluating the remaining expressions. For example, if (c1.var4 < var4) is likely to be true 99% of the time, you could move that to the top.

    Short of that, it seems a bit odd that you'd be getting a significant amount of time spent in this method unless these conditions hit a database or something like that.