One of the more simple code smells to look for is Selector Arguments.
A selector argument is generally a boolean flag that is passed to a method invocation. This boolean flag is then used to determine much if not all of the algorithm that is used when the method is invoked. Let's look at an example:
Notice how the boolean flag is the gateway for which of two completely separate algorithms are used in any given invocation. Look at each branch of the if statement. They are completely independent of each other. This selector argument is hiding the fact that we have two different algorithms stuck into the same method.
Let's consider an alternate layout
By moving the pieces to separate methods, we gain the benefit of better naming, more focused design, and more readability. Now the invoking method may be able to simply call the correct instance of the two methods. Or we can at the very least move to something like this:
It's better to invoke them directly, but this is still an improvement, as we have now cut the displayInvoice method down to a FAR more readable version, and extracted out a couple methods that declare their intent much better.
And from here we can even move to this:
But it's important to note that this isn't JUST about cutting the algorithm into pieces. The code smell isn't JUST about readability from the standpoint that the displayInvoice method is now more readable.
The REAL crux of this code smell is that we don't want to take two SEPARATE algorithms, two separate solutions, and stick them together into the same method. Each of the above algorithms is completely separate from the other. They DON'T BELONG together. Regardless of the fact that they're both display invoices. They have as much in common as a car and a plane (both things get you where you need to go, but they do it very differently).
When they're separate, keep them separate. Let your code SAY what it is. Identify your algorithms. Name them. Name them explicitly.
Look at it the other way. If we start with a well-named method like "displayHTMLInvoice" and we go to "displayInvoice" then the next step is "display" and eventually we get to a method named "doStuff". We don't want methods like "doStuff" in our applications. We want clear, expressive names and code.
This code smell helps us get there. Like all code smells it can be taken to unproductive extremes. It doesn't ALWAYS mean we have to change our code.
Let's consider another scenario: what if the algorithms are 90% the same, with a small difference based on the boolean parameter? Even in this case, if we are using a boolean parameter, it can still be beneficial to create two separate methods each named more explicitly. But as always, you need to consider the implications.
Enjoy this discussion? Sign up for our newsletter here.