“If” statements often attract complexity. They gather compound conditions that are hard to understand. More “if” statements get nested inside. It gets hard to know under what conditions any bit of code will run.
“Duplicate and Customize” is a refactoring that can help untangle these jumbles. One case is where the conditions are somewhat overlapping: in the picture above, imagine if the colored block also has conditions involving C1 or C2. By unwinding some of the logic, we get code that is specific to each condition. I also apply this refactoring when I want a particular condition to drive the overall decision, but that condition is buried inside the current structure.
Mechanics
- Find code that occurs under two or more conditions. (This refactoring is most useful when the code has multiple paths that apply under certain conditions.)
It could be a case like the picture above, an “if” statement with two conditions or’d together. Or perhaps it’s an extracted method called in a number of separate places, but the extracted method has conditional code.
- Duplicate the code in both places. (For an “if”, it may mean splitting an “or” into separate branches. For a method call, you may inline method.)
- Customize the code in each separate place.
You may see other refactorings made possible. For example, if you inlined a method, you may be able to extract a new method that handles the common parts.
Example – Gilded Rose
One portion of the Gilded Rose kata starts with code almost like this. We’ll do a bit of setup to make the pattern more obvious. [I’ll italicize where I’m going to change, and bold what I just did change.]
if (!items[i].name.equals("Aged Brie") && !items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].quality > 0) {
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].quality = items[i].quality - 1;
}
}
} else {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
}
}
We can apply DeMorgan’s law: (! C1 && ! C2) is the same as !(C1 || C2), then remove the “!” and flip the if and else clauses:
if (items[i].name.equals("Aged Brie") || items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
}
} else {
if (items[i].quality > 0) {
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].quality = items[i].quality - 1;
}
}
}
Now we have the C1 || C2 structure that was in the opening diagram, so duplicate the branches. Why not just change in place? Because duplicating the branches gives us smaller steps, and is less prone to error.
if (items[i].name.equals("Aged Brie")) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
}
} else if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
}
} else {
if (items[i].quality > 0) {
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].quality = items[i].quality - 1;
}
}
}
Now simplify each branch. That inner condition is clearly false in the first case, and true in the second. (I think if you replace the condition with false or true, IntelliJ will even offer to simplify it.) The key benefit shows up: both branches are simpler.
if (items[i].name.equals("Aged Brie")) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
} else if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
if (items[i].sellIn < 11) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
if (items[i].sellIn < 6) {
if (items[i].quality < 50) {
items[i].quality = items[i].quality + 1;
}
}
}
} else {
if (items[i].quality > 0) {
if (!items[i].name.equals("Sulfuras, Hand of Ragnaros")) {
items[i].quality = items[i].quality - 1;
}
}
}
There is still plenty of do (and I have no objection if you do that before this refactoring). But we’ve eliminated a level of nesting and some redundant checks. We’ve “pulled” the name condition to the outside.
Alternative Approach
Let me sketch out an alternative approach, that we might apply if we were explicitly trying to make name the outside condition.
- Extract the original code block above into a method
updateQuality().
:
updateQuality()
:
method updateQuality() {
// original code above
}
- Write an outer “if-else” statement covering all mutually exclusive possibilities; call the method in each branch.
if brie updateQuality() else if backstage pass updateQuality() else if sulfuras updateQuality() else updateQuality()
- Inline the method into one branch. (Not shown.)
- Carry the outer condition inward and look for obvious tautologies (always true) and contradictions (always false). This “pulls” the use of the condition to the top level.
(Be careful that nothing can change between the checks; if some of the statements incremented “i”, we’d be in trouble.)
Contradiction (always false):
In the brie case:
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert"))
We know it's not (because we know it is "Aged Brie"), so replace the condition with "if (false)", and simplify.
Tautology (always true):
In the backstage pass condition:
if (items[i].name.equals("Backstage passes to a TAFKAL80ETC concert"))
We know it is true, because of the (now) outer clause, so replace the condition with "if (true)", and simplify.
- Repeat for each outer branch.
Now the bad news: This is not a general solution. It applies in this case because name[i] cannot change in the body of updateQuality(). Compare if our new outer condition were based on items[i].quality; the value changes as we go.
Conclusion
“Duplicate and Customize” gives you a way to simplify code that uses a combination of conditions. It doesn’t apply everywhere, but it can be a quick way to reorganize conditions when it does.