ℹ️ This repository is part of my Refactoring catalog based on Fowler's book with the same title. Please see kaiosilveira/refactoring for more details.
Formerly: Parameterize Method
| Before | After |
|---|---|
export function tenPercentRaise(aPerson) {
aPerson.salary = aPerson.salary.multiply(1.1);
}
export function fivePercentRaise(aPerson) {
aPerson.salary = aPerson.salary.multiply(1.05);
} |
export function raise(aPerson, factor) {
aPerson.salary = aPerson.salary.multiply(1 + factor);
} |
There are only a few things in programming that are more annoying than two chunks of code that does almost the same thing, except for a tiny detail. This refactoring helps solving this issue.
A quick and easy example is the code snippet above, which deals with a salary increase: we have fivePercentRaise and tenPercentRaise, both of them identical in implementation unless for a different multiplying factor. To solve this issue, we can first introduce a more general raise function:
@@ -5,3 +5,7 @@
+export function raise(aPerson, factor) {
+ aPerson.salary = aPerson.salary.multiply(1 + factor);
+}And then call raise at fivePercentRaise and tenPercentRaise:
+++ b/src/salary-increase/index.js
@@ -1,9 +1,9 @@
export function tenPercentRaise(aPerson) {
- aPerson.salary = aPerson.salary.multiply(1.1);
+ raise(aPerson, 0.1);
}
export function fivePercentRaise(aPerson) {
- aPerson.salary = aPerson.salary.multiply(1.05);
+ raise(aPerson, 0.05);
}From there onwards, that'd be our choice whether or not to inline both functions at their callers, or keep them as is, optimizing for readability. The real world is a bit more complex, though, that's why we have a different, more involved example below.
Our working example is a program that calculates usage charges, based on three usage bands, each of which with their own rules and ranges. There are three functions:
export function bottomBand(usage) {
return Math.min(usage, 100);
}
export function middleBand(usage) {
return usage > 100 ? Math.min(usage, 200) - 100 : 0;
}
export function topBand(usage) {
return usage > 200 ? usage - 200 : 0;
}All of them used inside baseCharge:
export function baseCharge(usage) {
if (usage < 0) return usd(0);
const amount = bottomBand(usage) * 0.03 + middleBand(usage) * 0.05 + topBand(usage) * 0.07;
return usd(amount);
}Our goal here is to merge their behaviors into a single function.
The test suite for the program is straightfoward:
describe('baseCharge', () => {
it('should return 0 when usage is less than 0', () => {
expect(baseCharge(-1)).toEqual('$0.00');
});
it('should return an amount based on the three bands of usage', () => {
expect(baseCharge(0)).toEqual('$0.00');
expect(baseCharge(50)).toEqual('$1.50');
expect(baseCharge(100)).toEqual('$3.00');
expect(baseCharge(150)).toEqual('$5.50');
expect(baseCharge(200)).toEqual('$8.00');
expect(baseCharge(250)).toEqual('$11.50');
});
});
describe('withinBand', () => {
it('should return the middle band of usage', () => {
expect(withinBand(50)).toEqual(0);
expect(withinBand(100)).toEqual(0);
expect(withinBand(150)).toEqual(50);
expect(withinBand(200)).toEqual(100);
expect(withinBand(250)).toEqual(100);
});
});With our harness in place, it's time to start.
Since we want to generalize the code into a single place, we need a place to start. The optimal place here is middleBand, since it's not in either of the extremes. We start with the works by adding the bottom and top parameters to middleBand:
@@ -16,7 +16,7 @@ export function bottomBand(usage) {
return Math.min(usage, 100);
}
-export function middleBand(usage) {
+export function middleBand(usage, bottom, top) {
return usage > 100 ? Math.min(usage, 200) - 100 : 0;
}They do nothing at the moment, but will come in handy soon.
We then provide values for bottom and top to middleBand at baseCharge:
@@ -8,7 +8,8 @@ function usd(amount) {
export function baseCharge(usage) {
if (usage < 0) return usd(0);
- const amount = bottomBand(usage) * 0.03 + middleBand(usage) * 0.05 + topBand(usage) * 0.07;
+ const amount =
+ bottomBand(usage) * 0.03 + middleBand(usage, 100, 200) * 0.05 + topBand(usage) * 0.07;
return usd(amount);
}and rename middleBand to withinBand:
@@ -9,7 +9,7 @@ function usd(amount) {
export function baseCharge(usage) {
if (usage < 0) return usd(0);
const amount =
- bottomBand(usage) * 0.03 + middleBand(usage, 100, 200) * 0.05 + topBand(usage) * 0.07;
+ bottomBand(usage) * 0.03 + withinBand(usage, 100, 200) * 0.05 + topBand(usage) * 0.07;
return usd(amount);
}
@@ -17,7 +17,7 @@ export function bottomBand(usage) {
return Math.min(usage, 100);
}
-export function middleBand(usage, bottom, top) {
+export function withinBand(usage, bottom, top) {
return usage > 100 ? Math.min(usage, 200) - 100 : 0;
}Now things start to gain form. We start using the bottom and top parameters at withinBand:
@@ -18,7 +18,7 @@ export function bottomBand(usage) {
}
export function withinBand(usage, bottom, top) {
- return usage > 100 ? Math.min(usage, 200) - 100 : 0;
+ return usage > bottom ? Math.min(usage, top) - bottom : 0;
}
export function topBand(usage) {and replace a call to bottomBand with withinBand(usage, 0, 100) at baseCharge:
@@ -9,7 +9,7 @@ function usd(amount) {
export function baseCharge(usage) {
if (usage < 0) return usd(0);
const amount =
- bottomBand(usage) * 0.03 + withinBand(usage, 100, 200) * 0.05 + topBand(usage) * 0.07;
+ withinBand(usage, 0, 100) * 0.03 + withinBand(usage, 100, 200) * 0.05 + topBand(usage) * 0.07;
return usd(amount);
}Tests still work, so we move forward with removing the bottomBand function altogether:
@@ -13,10 +13,6 @@ export function baseCharge(usage) {
return usd(amount);
}
-export function bottomBand(usage) {
- return Math.min(usage, 100);
-}
-
export function withinBand(usage, bottom, top) {
return usage > bottom ? Math.min(usage, top) - bottom : 0;
}Then, we repeat the process for topBand. First, we replace a call to it with withinBand at baseCharge:
+++ b/src/usage-charge/index.js
@@ -8,8 +8,12 @@ function usd(amount) {
export function baseCharge(usage) {
if (usage < 0) return usd(0);
+
const amount =
- withinBand(usage, 0, 100) * 0.03 + withinBand(usage, 100, 200) * 0.05 + topBand(usage) * 0.07;
+ withinBand(usage, 0, 100) * 0.03 +
+ withinBand(usage, 100, 200) * 0.05 +
+ withinBand(usage, 200, Infinity) * 0.07;
+
return usd(amount);
}Then simply remove it completely:
@@ -20,7 +20,3 @@ export function baseCharge(usage) {
export function withinBand(usage, bottom, top) {
return usage > bottom ? Math.min(usage, top) - bottom : 0;
}
-
-export function topBand(usage) {
- return usage > 200 ? usage - 200 : 0;
-}And that's it! Now baseCharge uses only withinBand to apply the rules for the three different bands.
Below there's the commit history for the steps detailed above.
| Commit SHA | Message |
|---|---|
| e1a8c6f | add bottom and top parameters to middleBand |
| 611beda | provide bottom and top values to middleBand |
| a130a7b | rename middleBand to withinBand |
| 4e25e4a | use bottom and top parameters at withinBand |
| 6c69b22 | replace call to bottomBand with withinBand(usage, 0, 100) at baseCharge |
| 9225fba | remove bottomBand function |
| 36e8dc1 | replace call to topBand with withinBand at baseCharge |
| f52ca2c | remove topBand function |
For the full commit history for this project, check the Commit History tab.