avoid boolean parameters
You are tempted to create a method with a boolean argument. The caller will set the argument to true or false depending on which code path he wishes the called method to take. This is an extremely convenient way to add a small degree of configurability to an algorithm, and means that both paths through the called method can share setup and cleanup code.
class MyClass
void dosomething(boolean followPathA) {
// setup
if (followPathA)
// pathA
else
// pathB
// cleanup
}
However, the use of a boolean argument reduces readability. For example, in a method such as
workspace.createFile(name, true), what does “false” mean? It is much more intention-revealing to call a method such as workspace.createTempFile(name). Furthermore, boolean arguments always create duplication, either of code or of responsibility, and often of a very insidious nature. This is because the called method contains a runtime test to decide between two different code paths, despite thet fact that the calling method already knows which branch it requires! The duplication not only wastes a little runtime, it also spreads knowledge and responsibility further than is necessary. The resulting code is more closely coupled than the design it represents. These effects are magnified exponentially each time the boolean argument is passed on in a further method call.
Therefore avoid the need for a boolean argument by creating two versions of the method, in which the meaning of the boolean is reflected in the names of the new methods. For example, replace
File createFile(String name, boolean istemp)
by
File createFile(String name) File createTempFile(String name)
If the resulting new methods have similar logic, extract the duplicate code into shared methods that can be called by each.
(Here’s a fun story on a related theme.)
This is a modern take on the 1970s structured design concept of “control coupling”, held to be undesirable because it results in one method (”module” in those days) telling another what to do when it would be simpler to just do it itself. As you point out, this is always unnecessary, and results in a duplicated test and an unwanted dependency between the methods. But as well as this, the presence of a control flag suggests bad cohesion in the called method, as it needs to be told which of its multiple functions to perform. A method should do one thing, and do it well. If our imaginary designer had instead named the method after what it *really* did, e.g. createPermanentOrTemporaryFile(), then the cohesion problem would have been immediately apparent. In my youth, creating a control couple was a flogging offence! But I can’t say if all the old structured design guidelines transfer to the modern world. Some of the restrictions were due to the notion of hierarchy of control in a top-down design, e.g. it was held to be wrong for a module “lower down the hierarchy” to pass back a control flag to one “higher up”.
Colin Rutland
July 19, 2004 at 4:03 pm
I usually create a two-item enum or two differently-named methods wherever I would have used a boolean parameter. Recently I’ve been using VB.NET, which has named parameters, so whenever I call someone else’s code which has a boolean parameter, I can call it like this:
CreateFile(”/path”, IsTemp:=True)
Now I’m anxiously waiting for Ruby to support named parameters.
Rik Hemsley
April 7, 2005 at 8:43 am
Wayne’s World Methods
Excellent! … NOT!I’ve just been bitten by a common coding style: methods that take an argument which negates the meaning of the method’s name. This makes the implementor’s job easier but makes code that calls the method harder to read,…
Mistaeks I Hav Made
April 9, 2005 at 6:18 pm
Creating two different functions is fine when you only have one flag, but become a nightmare if you have more than one. For example a displayText function that can display text in any combination of bold, italic, underline and strikethrough would require 16 different functions – e.g. displayTextInBoldAndUnderline(…) etc. Having multiple functions with different names also makes it hard to change the behaviour at runtime. Here is a simple exercise: rewrite the above function into the 16 separate functions. Then write a code snippet that iterates through a list of strings and prints it in bold if it contains a ‘b’, underline if it contains a ‘u’, italic if it contains an ‘i’, and strikethrough if it contains an ’s’. Multiple combinations are allowed, of course.As Rik says, in a modern language that allows named parameters with default values this becomes a non-issue. Python has had this facility from the start, so you can call the function as: displayText(”some text”, bold=True, underline=True)
Dave Kirby
April 10, 2005 at 11:24 am
Responding to Rik:
I agree, many languages these days offer ways to render a boolean parameter more readable. However, that does nothing to remove the duplication I pointed out – that somewhere up the calling stack is another method that already knows which code path should be taken. As Colin points out, we should try to avoid that degree of interference and control.
Kevin Rutherford
May 16, 2005 at 9:35 pm
Responding to Dave:
I would probably tackle your displayText example using Decorators. That is, I would look for a way to create little “do it in bold” and “underline it” objects that could modify or add to the original behaviour of the text string in question. As I tried to say in hexagonal soup, the particular caller who knows which text decorations it needs will assemble the string together with the appropriate decorators at that time. The rest of the system need never know that the decorators are there. Little objects each doing one job, instead of functions doing several.
Kevin Rutherford
May 16, 2005 at 10:00 pm
Nico Mommaerts has posted related thoughts at http://www.hansei-kaizen.be/article/144/thissetdraftfalse-or-thispost
kevin
May 10, 2006 at 10:33 pm
I agree completely. In Dave’s bold/underline example, Kevin points out correctly that single functions to affect formatting in that way is probably not the ideal API, so in trying to convert said function with 4 booleans to 16 individual methods, we’ve discovered something that’s probably a code smell, which has value.
What’s more, is I’d argue that even though it’s more code and tedious for the author of the library, that the individual versions are still more readable. As an API consumer I’d still rather write:
displayTextInBoldAndUnderline()
then:
displayText(true, true, false, true);
There’s no way anyone reading the code later has a chance of understanding the 2nd one without checking out the displayText documentation, but the first one reads like English and is clear.
But Kevin’s right, fix the API, because it’s only a matter of time until someone wants that text to be able to marked superscript or subscript, and any sort of flagging technique is quickly going to get out of control.
Rob Meyer
June 26, 2006 at 8:24 pm
This strikes me as a perfect place to use a type-safe enumeration. Then instead of booleans, you could write something like this:
enum FileType { GenericFile, TemporaryFile }
File createFile(String name, FileType type);
// call like this:
File f = FileCreator.createFile("/my/file/path", FileType.GenericFile);
This also has another benefit over booleans, namely the ability to assume more than two values. You could easily extend the enumeration like this:
enum FileType { GenericFile, TemporaryFile, Directory }
Not that this would be the best way to create a directory, of course, but it’s more straightforward than this:
File createFile(String path, boolean isTemp, boolean isDir);
Brian
June 7, 2007 at 8:45 pm
@Brian:
I agree that your enums are more readable (and extensible) than the booleans. But they still suffer from the problem of duplication. The code inside the method must include conditional structures to figure out what to do. And those conditionals duplicate knowledge that’s already present in the method’s caller.
And that, in turn, means that the system as a whole is harder to maintain. Because each time we want to add a new file type, for example, we’ll have to change and/or recompile many different classes.
So it seems to me that forcing all conceptually related behaviour (creating new things in the filestore) through this one interface makes us work too hard. We have to work to make it readable, and we have to work harder when things change. Which is why I have this little rule of thumb – when I feel the need to introduce a Boolean parameter, I take a step back and re-think my object design, before it’s too late.
Kevin Rutherford
June 8, 2007 at 7:38 am
[...] remained popular throughout – and indeed the stats show there’s been a lot of interest in avoid boolean parameters and if… just recently – both written in July of 2004. Those two, together with the product [...]
3 years old today! « silk and spinach
June 16, 2007 at 7:42 pm
Kevin, your comments are correct and so the key issue here is that the system somewhere know the decision; Here I got a question for you, hope you show me how to avoid the if else use pattern or some smart ideas:
say I got input from user which is string, for example “Engineer”, “Manager”, “Saleman” etc, in my program once I get this string, I should create object of this class accordingly, I am using C++, so the straigtforward way to do it as :
if(name == “Engineer”)
return new Engineer();
else if(name == “Manager”)
return new Manager();
esle if if(name == “Saleman”)
return new Saleman();
else
return null;
I spent quite a while to see if I can use subclass to replace
the type which is string here, but still I have to swith the decision to someone else, to create concret class of type;
Do you have any idea? Thanks
jack zhang
May 16, 2008 at 12:25 am
Hi Jack, The key is that this string is input from the user. If you consider your software in terms of the hexagonal architecture pattern, the code you show would sit in an Adapter. The job of the adapters is to convert between the domain objects and the real world — in this case to convert between the user’s string and an object of the equivalent class. So it’s good code (as long as it occurs only once in the system).
Kevin Rutherford
May 19, 2008 at 8:50 pm
I think you can extend this pattern further to any parameter, be it a boolean, enumeration, or whatever, that is used inside the algorithm to branch. I started doing this earlier this week at my workplace, but immediately stopped because I knew it smelled. I applaud your articulation of this code smell.
MoffDub
October 26, 2008 at 2:09 am
@Moffdub: You’re right — as noted by Colin in the first comment above, a boolean parameter is a special case of the Control Coupling smell. There’s a section describing Control Coupling in my imminent book, the Ruby Refactoring Workbook.
Kevin Rutherford
October 27, 2008 at 8:32 am
[...] avoid boolean parameters [...]
the anti-if campaign « silk and spinach
October 28, 2008 at 11:19 am
[...] main news is the arrival of a new smell: I’ve included a first naive check for the Control Couple [...]
reek 0.3.0 released « silk and spinach
November 2, 2008 at 10:24 pm
[...] editing in Rails 2.0setup and teardown for a ruby TestSuitea ruby sparkline showing variationavoid boolean parametersreek, a code smells detector for rubythe anti-if campaignnothing but "uses" and [...]
2005 silk and spinach top ten « silk and spinach
June 30, 2009 at 3:36 pm