DEV Community

Roland Weisleder
Roland Weisleder

Posted on • Originally published at blog.rweisleder.de

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();
}
Enter fullscreen mode Exit fullscreen mode

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);
}
Enter fullscreen mode Exit fullscreen mode

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
    }
    ...
}
Enter fullscreen mode Exit fullscreen mode

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

Document[] documents = documentService.loadDocuments(false);
Enter fullscreen mode Exit fullscreen mode

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);
}
Enter fullscreen mode Exit fullscreen mode

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

public interface DocumentService {

    Document[] loadPdfDocuments();

    Document[] loadTiffDocuments();

    Document[] loadRtfDocuments();
}
Enter fullscreen mode Exit fullscreen mode

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

Document[] documents = documentService.loadDocuments(DocumentType.PDF);
Document[] documents = documentService.loadPdfDocuments();
Enter fullscreen mode Exit fullscreen mode

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;
    }
}
Enter fullscreen mode Exit fullscreen mode

to

public class Action {

    private boolean enabled;

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

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

Enter fullscreen mode Exit fullscreen mode

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

action.setEnabled(actionCheckBox.isChecked());
Enter fullscreen mode Exit fullscreen mode

and

if (actionCheckBox.isChecked()) {
    action.enable();
} else {
    action.disable();
}
Enter fullscreen mode Exit fullscreen mode

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

Further Reading

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

Top comments (5)

Collapse
 
jmfayard profile image
Jean-Michel πŸ•΅πŸ»β€β™‚οΈ Fayard • Edited

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.

"Nullable booleans" with three values are an offense to the mathematical laws of boolean algebras.
Booleans can by definition only have two values.
Please don't do that.

Instead use an enum with three values

enum class God { 
   Father, 
   Son,
   HolySpirit,
}
Enter fullscreen mode Exit fullscreen mode
Collapse
 
jmfayard profile image
Jean-Michel πŸ•΅πŸ»β€β™‚οΈ Fayard • Edited
Document[] documents = documentService.loadDocuments(false);
Enter fullscreen mode Exit fullscreen mode

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?

Yes I can because I would be calling the API from Kotlin who has named (and default) parameters

val documents = documentService.loadDocuments(loadTiff = false);
Enter fullscreen mode Exit fullscreen mode

Ok that's a bad faith argument, you can do the same in Java by using the (good but cluncky) Builder pattern, and I don't actually like non obvious boolean parameters either.

Collapse
 
user1111333 profile image
Sacred (void*)

Great article, thanks for sharing.

I would go for something like following

public default loadDocuments(DocumentType arg_type){
    EnumSet<DocumentType> _types=EnumSet.of(arg_type);
    return loadDocuments(_types);
}
public loadDocuments(EnumSet<DocumentType> arg_types);
Enter fullscreen mode Exit fullscreen mode
Collapse
 
dariusv_65 profile image
darius-v

Some people argue - add the arguments to an object and there will be no boolean. I think it still is, just hidden.

`loadDocuments(UserInput $userInput)
{
if ($userInput->loadTiff) {
return generateTiff();
}

return generatePdf();
}`

vs
`
loadDocuments(bool loadTiff)
{
if (loadTiff) {
return generateTiff();
}

return generatePdf();
}
`

UserInput will have bool flag. How this makes you not go to look at definition what it is doing?

And how will it then not violate single responsibility principle.
And is here priniple violated? Generating document is dedicated to some other function like generatePdf(). I heard then it does not count as loadDocuments doing many things.

Collapse
 
manuartero profile image
Manuel Artero Anguita 🟨 • Edited

Hi @dariusv_65 ! good points.

syntax tip: change those Β«commasΒ» using three instead for code blocks:

loadDocuments(bool loadTiff) {
  if (loadTiff) {
    return generateTiff();
  }
  return generatePdf();
}
Enter fullscreen mode Exit fullscreen mode