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 patternJohn 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.