Martin Fowler describes the Flag Argument code smell as a “kind of function argument that tells the function to carry out a different operation depending on its value”.
Answers to Is it wrong to use a boolean argument to determine behavior? seem to conflate all boolean arguments with flag arguments. I think that question doesn’t fully explore the problem of boolean arguments.
However, I have a feeling that not all boolean arguments are equal.
A use-case for boolean arguments
Imagine a popup that shows a message that is partly dependant on whether a user is logged in or not. We would define a method
show(boolean isLoggedIn)
The method would be called in this way:
popup.show(session.isLoggedIn())
It seems to me that in this example readability and cohesion are not impacted negatively. Furthermore, I don’t think that this is a flag argument. It doesn’t tell the method how exactly to behave, but conveys the information necessary for the method to make that decision.
Alternatives
I can’t think of a way to improve this method using the solutions proposed in the discussion above.
Enum would look something like this:
popup.show(session.isUserLoggedIn()
? Popup.ContentType.LoggedIn
: Popup.ContentType.LoggedOut);
Splitting the method would look like this:
if (session.isUserLoggedIn()) {
popup.showLoggedInMessage();
} else {
popup.showLoggedOutMessage();
}
This seems longer and also leads to code duplication if they have common parameters:
if (session.isUserLoggedIn) {
popup.showLoggedInMessage(commonIntro);
} else {
popup.showLoggedOutMessage(commonIntro);
}
In my opinion, using a boolean argument is better than both of these approaches.
So, are all boolean arguments a code smell, or can they be a valid technique?
6
No rule is absolute in software development and everything depends on context.
That being said, I would not consider your example to be necessarily a good case for boolean arguments. An API like show(bool isLoggedIn) does not tell me what is going to happen without me reading documentation or code, whereas functionality of show(string messageToDisplay) would be quite obvious from the signature.
Are boolean arguments a code smell? Yes. As it is something “that possibly indicates a deeper problem”. That does not mean they should never ever be used, just that you should consider twice before using them.
The possible problem of having a bool flag is that it can be too easily (and unintentionally) abused.
Lets consider a toy example. Say you have a picture drawing application that everyone loves but is only able to save images to bmp. The CEO comes in one day and says that the users would love to have an option of saving to jpeg as well. You, of course, agree to do this and decide to do it in the simpless way possible – adding a bool flag to your SavePicture() method. Why? Well, probably because the SavePicture method does many things – collects the pixel values, reads the file name that the user entered, encodes the picture to get an array of bytes and finally writes everything to disk. 3 of 4 of these saving tasks have nothing to do with picture formats, only the encoding step does, so its much easier to put an if statement in the middle that checks agains a bool instead of refactoring the method. And this is sort of fine until the CEO asks to put in a new png format 3 months later.
This is where the bool starts to bite. Instead of splitting the code to 4 separate functions early on and using them where appropriate, you have increased the complexity of the SavePicture(asJpeg) method with intertwined ifs. Whats worse, the CEO might ask some other person to implement the new feature, someone who has never seen how SavePicture forks. Now, with the bool added, it will be harder for him to unravel the logic in order to refactor, so he’ll probably change your bool to an enum or even add a second bool.
All in all, a bool is usually a smell of smells. If it’s easier to add a bool, its probably because the method is hundreds of lines long and the parts of your system do not compose well and cannot be easily rearranged.
1
In Clean Code by Robert Martin, we see that in general when we have a boolean as argument, it usually indicates that your method is doing more than one thing. By this, I mean, you are going against the Single Responsibility Principle.
If your method’s job is to check the flag and perform an operation based on its value, then your code should be considered clean.
While thinking about code smells and clean code, you should always be thinking about the SOLID principles. They are a good guide to decide what design or how your code should look like.
First, Martin seems to differentiate between flags, or Boolean variables that change the way a function behaves, and Boolean variables that are simply set or passed by a function.
I interpret Martin Fowler’s argument against flag arguments to be a balance between writing two separate functions for each use case that are essentially the same, and one function for two use cases that are very different. You don’t want to constantly check the flag in your function or have the same function behave completely differently when a flag input is changed, but you also have to make exceptions when splitting into two functions would result in unnecessary duplication. Too many flag arguments can lead to constant worrying about whether the function is handling all its states correctly and lack of clarity about its behavior and purpose, while eliminating flag arguments overzealously can lead to unnecessary duplication.
For example, if your code for show(boolean isLoggedIn) looks like this:
if (isLoggedIn) {
updateBar();
} else {
updateFoo();
}
createWindow();
if (isLoggedIn) {
addModal()
}
fillInRestOfWindow()
Then you should probably split it into two functions. Sure, the caller is more complicated because now it needs to include an if statement, but you’ve erased several if statements, and removed complexity from the function by not having to juggle several states at once.
Like all code smells, it merely indicates a pattern, which if over-used, will lead to bug-prone programming. Yet judicious use, with awareness of the code smell, is ok.