Flag pattern

John works as a software developer for a company that produces coffee machines. He develops the controller software that runs on the machine. Basically, the main function of the software looks like this:

public class Controller {
    private Reservoir mReservoir;
    private Heater mHeater;
    
    public void StartCycle() {
        mHeater.Start();
        mReservoir.WaitEmpty();
    }
}

This main function controls the current version of the coffee machine that is in production. This is a simple machine with a single start button, that uses ground beans. The user puts the required amounts of beans and water in the machine and then presses the start button. The software controller starts the heater, then waits until the water reservoir is empty and assumes that coffee is ready by then.

One day, John needs to adapt the software to support the new model machine, which has built-in dispensers for water and ground beans. Pressing the start button on this machine will dispense water and ground beans for one cup, then continue the coffee making process as previously.

Read more: Flag pattern

John adapts the controller as follows:

public class Controller {
    private bool mHasDispensers;
    private Dispenser mBeansDispenser;
    private Dispenser mWaterDispenser;
    private Reservoir mReservoir;
    private Heater mHeater;
    
    public void StartCycle() {
        if(mHasDispensers) {
            mBeansDispenser.Dispense();
            mWaterDispenser.Dispense();
        }
        mHeater.Start();
        mReservoir.WaitEmpty();
    }
}

Everything works fine. But another day, the company wants to introduce a cheaper version of the basic model, without a sensor in the water reservoir. Instead, the Controller must turn on heater for a fixed amount of time.

So John makes the next adaptation:

public class Controller {
    private bool mHasDispensers;
    private bool mHasReservoirEmptySensor;
    private Dispenser mBeansDispenser;
    private Dispenser mWaterDispenser;
    private Reservoir mReservoir;
    private Heater mHeater;
    
    public void StartCycle() {
        if(mHasDispensers) {
            mBeansDispenser.Dispense();
            mWaterDispenser.Dispense();
        }
        mHeater.Start();
        if(mHasReservoirEmptySensor) {
            mReservoir.WaitEmpty();
        } else {
            const int cHeatingTimeMs = 60_000;
            Thread.CurrentThread.Sleep(cHeatingTimeMs);
        }
    }
}

Even if there is no sensor to detect an empty reservoir, the reservoir itself is still there for other uses. So a null check is not possible.

As you can see, the function grows and grows. The software must serve more variations of machines and that necessitates increase in code size. However, there are severe drawbacks by writing the code this way.

The code becomes difficult to comprehend, and therefore it is easy to introduce bugs. It is also difficult to reason about the code. As an example, the code doesn’t tell you that the simplest machine has no dispensers. In fact, the software suggests a wider range of machines than actually exist. But the most important point is that the essence (basically the simple flow of dispense – heat – wait) is buried in details. Each new if..else construct makes this problem worse.

The problem of the flag pattern

The main source of the problem is the use of the anti-pattern called the Flag pattern. The fields mHasDispensers and mHasReservoir are examples of this anti-pattern. Wherever there is a boolean like this, there needs to be an if..else construct to act accordingly. And those if..else constructs introduce the complexity in the method. It won’t take many iterations before John will need to check combinations of flags, resulting in an explosion of possible combinations.

What is actually happening is that the StartCycle method starts to accumulate responsibilities that it actually should delegate to other classes. Those classes are not yet there and so their implementation becomes part of the StartCycle method. This makes StartCycle more and more complex.

Or, put it differently, StartCycle knows a lot of things that it shouldn’t. For example, it knows a lot about how the different machines work and how the options work. Removing this knowledge from StartCycle might make the code easier to read, easier to maintain and more flexible.

Usually, this complexity increases gradually with each change, while resolving it requires quite some amount of work, especially when not treated early. The problem is also that those flags are passed around, so resolving this means that all if..else constructs everywhere need to be refactored.

Variations of the flag pattern

The flag pattern occurs in a number of variations.

  • The boolean parameter: inside the function you’ll find an if(parameter)..else construct.
  • A class that exposes a boolean property that other classes use for decisions (and a decision is an if..else construct).
  • Object type checking: int heatingTime = (mHeater is TurboHeater) ? 30_000 : 60_000. This can be seen as a flag in disguise as it shows the same characteristics: it is easy to introduce and a sign that the functionality is in the wrong place. This one is easy to disguise: pass the object, but deep down the stack use it for type checking.
  • Enums, when not used at the boundary of the application. In the application layer, the only way to use enums are switch statements in every place.

Reducing complexity

Resolving the flag anti-pattern requires some refactoring. What works mostly depends on the context and how tangled the implementation is.

For example, to resolve the flags in this method:

public void DoSomething(bool thisWay, bool doAlsoSomethingElse) {
    if(thisWay) 
        DoThisWay();
    else 
        DoThatWay();
    
    if(doAlsoSomethingElse)
        DoSomethingElse();
}

Normally, the then and else blocks consist of more than one statement, so the very first step would be to extract methods from them.

The next step would be to change all call sites, by basically inlining the DoSomething method. For example, if DoSomething was called with thisWay = true and doAlsoSomethingElse = true, then replace the call by: DoThisWay(); DoSomethingElse();. (Practically speaking this is often more difficult since the thisWay parameter may be a variable that is passed down ten levels deep. But the idea stays the same.)

Resolving the flags in the Controller

If John wants to improve the readability of the code, the details must be removed from StartCycle. That would make the main flow clear again. John may refactor the controller as follows.

Introduce an interface to contain the Dispense() call, so that StartCycle can call it without knowing how it is implemented:

public interface IDispenser { 
    void Dispense() 
}

public class BuildInDispensers : IDispenser {
    ...
    public void Dispense() {
        mBeansDispenser.Dispense();
        mWaterDispenser.Dispense();
    }
}

public class NoDispenser : IDispenser {
    public void Dispense() {
        // empty
    }
}

public class Controller {
    ...
    public void StartCycle() {
        mDispenser.Dispense();
        mHeater.Start();
        if(mHasReservoirEmptySensor) {
            mReservoir.WaitEmpty();
        } else {
            const int cHeatingTimeMs = 60_000;
            Thread.CurrentThread.Sleep(cHeatingTimeMs);
        }
    }
}

As you see, the first if statement in StartCycle has gone. It has moved to the code that creates a new Controller, where it must figure out which IDispenser to provide to the Controller constructor. Chances are that the same if statement was already there, because the beans and water dispenser objects had to be passed to Controller anyway, so they had to be constructed for the specific machine types.

John can do the same for the waiting stage: introduce an interface with two implementations and use that in the Controller:

public interface IProcessor {
    void Process();
}

public class WaitReservoirEmpty : IProcessor {
    ...
    public void Process() {
        mReservoir.WaitEmpty();
    }
}

public class WaitTimeout : IProcessor {
    public void Process() {
        const int cHeatingTimeMs = 60_000;
        Thread.CurrentThread.Sleep(cHeatingTimeMs);
    }
}

public class Controller {
    ...
    public void StartCycle() {
        mDispenser.Dispense();
        mHeater.Start();
        mProcessor.Process();
    }
}

As you can see, the StartCycle implementation shows exactly the stages of the process, and nothing more. All details of the different machines have been moved to other classes.

Is this new code better? For such a small application, that’s questionable. But for large applications this design is much better, even though it requires more classes and interfaces. I think it is better for these reasons:

  • The classes themselves and their methods are each focusing on exactly one specific goal. Responsibilities are now clearly separated.
  • The cyclomatic complexity (number of paths through the code) is much lower. This means that testing is much easier, but also understanding the code is easier.
  • Adding new functionality is much easier. For example, adding another dispenser type means essentially just creating a new IDispenser implementation. You can be sure that this won’t affect existing functionality, so testing becomes much easier.

Leave a Reply

Your email address will not be published. Required fields are marked *