The Flag Parameter Anti-Pattern

While implementing a new feature, I came across the flag parameter anti-pattern in two unrelated places. I would like to take this as an opportunity to take a closer look at this anti-pattern.

A simple example

Lets assume we want to load all PDF documents via an API method. This could look like this:

public interface DocumentService {

    Document[] loadDocuments();
}

It Starts Smelling

In the next step we need a method to load all TIFF documents. Since the difference in implementation might be only one line of code, the result might look like this:

public interface DocumentService {

    /**
     * @param loadTiff true: loads TIFF documents - false: loads PDF documents
     */
    Document[] loadDocuments(boolean loadTiff);
}

The implementation is also modified very quickly with an additional if block:

public Document[] loadDocuments(boolean loadTiff) {
    int format = 1; // load PDF documents by default
    if (loadTiff) {
        format = 2; // except when TIFFs are requested
    }
    ...
}

However, if we look at the client side of this API, we already notice an unpleasant smell:

Document[] documents = documentService.loadDocuments(false);

At this code point, can you tell what the meaning of false is without looking at the definition of the method? And what happens if we change this to true?

The Smell Begins to Stink

In the next step we also need a method to load RTF documents. Again, the difference in implementation is only one line of code. But how can we reuse the existing method without having to re-implement the logic? The flag parameter stands in our way! The boolean parameter can only take two states, true or false. (Okay, the clever Java developer changes this to a Boolean parameter and has true, false and null as choices).

However, the flag parameter also shows us something else: the single responsibility principle is violated here. In fact, this method currently has more than one responsibility. On the one hand, it knows how to load PDF documents. On the other hand, it also knows how to load TIFF documents. Or more generally: such a method does two different things - once on true and once on false.

Possible Solutions

Spontaneously I can think of two simple ways to get rid of the flag parameter.

Option 1: We define an interface for the document types to query for information. Then we can generalize the method by using the document type as a parameter. However, there is a possibility of unnecessary over-generalization of the business logic.

public interface DocumentService {

    Document[] loadDocuments(DocumentType documentType);
}

Option 2: We create a specific method for each possible domain context.

public interface DocumentService {

    Document[] loadPdfDocuments();

    Document[] loadTiffDocuments();

    Document[] loadRtfDocuments();
}

In both cases, from the client's side, this results in readable code:

Document[] documents = documentService.loadDocuments(DocumentType.PDF);
Document[] documents = documentService.loadPdfDocuments();

What About Setter Methods?

If we were strict, then we would also have to change

public class Action {

    private boolean enabled;

    public void setEnabled(boolean enabled) {
        this.enabled = enabled;
    }
}

to

public class Action {

    private boolean enabled;

    public void enable() {
        this.enabled = true;
    }

    public void disable() {
        this.enabled = false;
    }
}

However, we don't have to be more papal than the pope here. If I had to choose between

action.setEnabled(actionCheckBox.isChecked());

and

if (actionCheckBox.isChecked()) {
    action.enable();
} else {
    action.disable();
}

for the client code, I would stay with the first one.

Further Reading

• FlagArgument by Martin Fowler
• Clean Code Tip: Eliminate Boolean Arguments by Uncle Bob

I hope this article was helpful to you. Feel free to leave a comment. For more Java stuff, follow me on Twitter.