What are we going to do?
Take a function that's written terribly and refactor (clean) it.
The Dirty Function 🦨
public void updateQuality() {
for (int i = 0; i < items.length; i++) {
if (!items[i].name.equals("Aged Brie") // branch 1
&& !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 { // branch 2
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;
}
}
}
}
}
}
}
Analysis of the code 🕵🏾
We notice that we have two main execution paths
- Marked by branch 1 and branch 2
Let's look at the first branch where we have three if statements.
- One with two boolean checks
- The other two has only one
We check four booleans to determine if a block of code is going to be executed or not.
So, we should abstract the boolean checks into a function and the code block into a function as well.
The code block decreases the quality field value, so we give the boolean checks function a name that suits it
(conditionsAreMetForQualityReduction)
As for the second branch, we can see that the ongoing operations are increasing the value of the quality field. So, we can extract this branch to
a function and call it something like (performQualityAdditionProcess)
The Refactored (Clean) Version 🧽
public void updateQuality() {
for (Item item : items) {
if (conditionsAreMetForQualityReduction(item))
item.quality = item.quality - 1;
else if (item.quality < 50)
performQualityAdditionProcess(item);
}
}
private void performQualityAdditionProcess(Item item) {
item.quality = item.quality + 1;
if (item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.sellIn < 11 && item.quality < 50) {
item.quality = item.quality + 1;
}
if (item.sellIn < 6 && item.quality < 50) {
item.quality = item.quality + 1;
}
}
}
private boolean conditionsAreMetForQualityReduction(Item item) {
String name = item.name;
return !name.equals("Aged Brie") &&
!name.equals("Backstage passes to a TAFKAL80ETC concert") &&
item.quality > 0 &&
!name.equals("Sulfuras, Hand of Ragnaros");
}
What did we do? 🙌
- Aggregate boolean checks that belong to the same check to a single expression.
Extract complex boolean expression to a function
Extract complex if/else bodies to functions.
Give the functions useful name that states their purpose
Note: By complex, I mean anything beyond one line of code.
Top comments (0)