There are many ways that code can go bad, but few of them are as infuriating as excessively long methods. In some cases, long methods aren't much of a problem. The method in Listing One is a good example. The method is long but it has distinct sections and minimal sharing of state between them. Because of this, it is relatively easy to reason about the various sections of the code. One could argue that, conceptually, this is really more than one method — it's just that no one has bothered to make that that apparent in the code as yet, by naming and extracting the sections.

Listing One

void processEvent(Event event, AuxDataContainer container) {
    if (event.getType() == GF_HOME) {
        int delay = event.getTime() + dispatcher.getLeadTime();
        container.acceptDelay(event, delay);
    }

    if (event.getType() == GF_REMOTE) {
        dispatcher.dispatch(new RemoteEvent(event));
    }    

   if (event.getType() == GF_NEIGHBORHOOD) {
      int errantFlag = event.getLoopBackFlag () | 0x10;
      dispatcher.dispatch(new RemoteEvent(event, errantFlag));
      int delay = event.getTime() + dispatcher.getLeadTime() - NEIGHBOR_ADVANCE;
      container.acceptDelay(event, delay);
   }

   if (event.getType() == GF_UK && container.isLocal()) {
        ...
   }

   if (event.getType() == GF_US || container.isLocalDispatch()) {
       …
   }   
}

Other methods, however, are truly nightmarish. In the worst cases, the logic of the method is obscured by deep, snarled nesting. Listing Two is a short, tame example of this sort of pathology.

Listing Two

int dispatchTrigger(Trigger trigger, Configuration configuration, boolean isFallback) {
    if (isFallBack && !trigger.clear()) {
        if (configuration.getSetting(MUTEX_DISPATCH) && trigger.getType() == ONE_SHOT) {
            if (trigger.atType() == PERIODIC) 
                trigger.fire();
            else 
                trigger.registerPoll();
        } else {
           int port = configuration.getIntSetting(DISPATCH_PORT);
           trigger.suppress(port);
           return MULTI_TRIGGERS; 
       }
    }
    return TRIGGER_OKAY;
}

It is often extremely hard to reason about code like this. You can see what computation happens at particular points in the code, but often the conditions under which it is executed are a complete mystery. In particularly bad cases, methods like this are called as part of an event loop and they set conditions that will be checked and acted upon on the next call.

It's tempting to think that automated refactoring tools offer a solution to this problem. They can indeed, but support for automated refactoring is all over the map in the industry. Tools for Java and C# are quite advanced. Tools for C++ lag behind due to the complexity of the language. Regardless of the language, it is easy to imagine valid refactorings that are not supported by current tooling. One of them is a refactoring I like to apply when I have these deeply snarled complex methods. I call it "Flatten Conditionals."

The idea behind flattening conditionals is to make obvious the conditions under which code executes. We do this through a sequence of steps that remove nesting and push conditions toward the top level of the method.

We can try it out for the code in Listing Tow, but first let's take a look at a couple of smaller pseudocode examples that demonstrate the mechanics.

In Listing Three moving all of the conditions to the top level is rather straightforward, the nested if statement can be seen as a simple conjunction of its expression and the expression in the outer if:

Listing Three

If (a && b) {
    X;
  If (c && d)  {
     Y;
  }
 Z;
}

Listing Four shows the result of the refactoring.

Listing Four

if (a && b) {
  X;
}
if (a && b && c && d) {
  Y;
}
if (a && b) {
  Z;
}

Else statements can be transformed with negation. Listing Five shows the code before the refactoring and Listing Six shows it afterward.

Listing Five: Code to be cleaned up.

if (a && b) {
    X;
    if (c && d) {
        Y;
    } 
    else {
        Z;
    }
}

Listing Six: The first clean-up step from Listing Five.

if (a && b) {
    X;
}

if (a && b && c && d) {
    Y;
}

if (a && b && !(c && d)) {
     Z;
}

At this point, the code in Listing Six might not seem to have gained much. The conditions we have are rather obscure and they contain quite a bit of duplication. But, code in this form can be more tractable. For one thing, it has the benefit of locality. The entry condition for each piece of code is right above it.