ℹ️ 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: Inline Method
Before | After |
---|---|
function getRating(aDriver) {
return moreThanFiveLateDeliveries(driver) ? 2 : 1;
}
function moreThanFiveLateDeliveries(driver) {
return driver.numberOfLateDeliveries > 5;
} |
function getRating(aDriver) {
return driver.numberOfLateDeliveries > 5 ? 2 : 1;
} |
Inverse of: Extract Function
Sometimes a short function's body is as clear as its name (and some other times, the function name is even longer than the one-line implementation of the code). In these cases, inlining the function statement and deleting the function seems natural, as we will often be using language constructs to better express the intent of the code, instead of hiding it behind function calls.
As our working examples, we will have two functions: one that calculates the rating of a driver based on their number of late deliveries, and another one that creates report lines for a given customer. These functions are currently calling one other function each, and we want to inline these calls.
As described above, the getRating
function is a simple method that calculates the rating of a given driver. It relies on another function called moreThanFiveLateDeliveries
to check whether the given driver has more than five late deliveries. We want to remove moreThanFiveLateDeliveries
and inline it in the body of getRating
.
The test suite For rating.js
is straightforward:
describe('rating', () => {
it('should return 1 if the driver has less than five late deliveries', () => {
const driver = { numberOfLateDeliveries: 4 };
const rating = getRating(driver);
expect(rating).toEqual(1);
});
it('should return 1 if the driver has five late deliveries', () => {
const driver = { numberOfLateDeliveries: 5 };
const rating = getRating(driver);
expect(rating).toEqual(1);
});
it('should return 2 if the driver has more than five late deliveries', () => {
const driver = { numberOfLateDeliveries: 6 };
const rating = getRating(driver);
expect(rating).toEqual(2);
});
});
As we want to inline the statement inside moreThanFiveLateDeliveries(dvr)
, first we need to rename its parameter so it has the same name as the one being passed down by the caller:
diff --git a/rating.js b/rating.js
@@ -2,8 +2,8 @@ function getRating(driver) {
return moreThanFiveLateDeliveries(driver) ? 2 : 1;
}
-function moreThanFiveLateDeliveries(dvr) {
- return dvr.numberOfLateDeliveries > 5;
+function moreThanFiveLateDeliveries(driver) {
+ return driver.numberOfLateDeliveries > 5;
}
module.exports = { getRating };
And then we inline the call to moreThanFiveLateDeliveries
:
diff --git a/rating.js b/rating.js
@@ -1,5 +1,5 @@
function getRating(driver) {
- return moreThanFiveLateDeliveries(driver) ? 2 : 1;
+ return driver.numberOfLateDeliveries > 5 ? 2 : 1;
}
function moreThanFiveLateDeliveries(driver) {
Finally, if there are no other references to moreThanFiveLateDeliveries
, we can safely remove it:
diff --git a/rating.js b/rating.js
@@ -2,8 +2,4 @@ function getRating(driver) {
return driver.numberOfLateDeliveries > 5 ? 2 : 1;
}
-function moreThanFiveLateDeliveries(driver) {
- return driver.numberOfLateDeliveries > 5;
-}
-
module.exports = { getRating };
See below a chronology (from top to bottom) of all the refactoring steps:
Commit SHA | Message |
---|---|
2608e81 | rename moreThanFiveLateDeliveries parameter |
3fe61cc | inline moreThanFiveLateDeliveries call |
33dd6b6 | remove moreThanFiveLateDeliveries function |
The full commit history can be seen in the Commit History tab.
The Initial code for report-lines.js
looks like this:
function reportLines(aCustomer) {
const lines = [];
gatherCustomerData(lines, aCustomer);
return lines;
}
function gatherCustomerData(out, aCustomer) {
out.push(['name', aCustomer.name]);
out.push(['location', aCustomer.location]);
}
The test suite for report-lines.js
is also straightforward:
describe('reportLines', () => {
it('should return the name and location lines for a given customer', () => {
const aCustomer = { name: 'Kaio', location: 'Lisbon' };
const result = reportLines(aCustomer);
expect(result).toHaveLength(2);
const [nameField, nameValue] = result[0];
expect(nameField).toEqual('name');
expect(nameValue).toEqual(aCustomer.name);
const [locationField, locationValue] = result[1];
expect(locationField).toEqual('location');
expect(locationValue).toEqual(aCustomer.location);
});
});
Similar to what we've done in the previous example, we start by renaming the first parameter:
diff --git a/report-lines.js b/report-lines.js
@@ -4,9 +4,9 @@ function reportLines(aCustomer) {
return lines;
}
-function gatherCustomerData(out, aCustomer) {
- out.push(['name', aCustomer.name]);
- out.push(['location', aCustomer.location]);
+function gatherCustomerData(lines, aCustomer) {
+ lines.push(['name', aCustomer.name]);
+ lines.push(['location', aCustomer.location]);
}
module.exports = { reportLines, gatherCustomerData };
And then we can be as careful as possible and move one statement to the caller at a time. We first move the aggregation of the name
field:
diff --git a/report-lines.js b/report-lines.js
@@ -1,11 +1,11 @@
function reportLines(aCustomer) {
const lines = [];
+ lines.push(['name', aCustomer.name]);
gatherCustomerData(lines, aCustomer);
return lines;
}
function gatherCustomerData(lines, aCustomer) {
- lines.push(['name', aCustomer.name]);
lines.push(['location', aCustomer.location]);
}
Then we move the aggregation of the location
field:
diff --git a/report-lines.js b/report-lines.js
@@ -1,12 +1,11 @@
function reportLines(aCustomer) {
const lines = [];
lines.push(['name', aCustomer.name]);
+ lines.push(['location', aCustomer.location]);
gatherCustomerData(lines, aCustomer);
return lines;
}
-function gatherCustomerData(lines, aCustomer) {
- lines.push(['location', aCustomer.location]);
-}
+function gatherCustomerData(lines, aCustomer) {}
module.exports = { reportLines, gatherCustomerData };
Finally, we can delete gatherCustomerData
:
diff --git a/report-lines.js b/report-lines.js
@@ -2,10 +2,8 @@ function reportLines(aCustomer) {
const lines = [];
lines.push(['name', aCustomer.name]);
lines.push(['location', aCustomer.location]);
- gatherCustomerData(lines, aCustomer);
+
return lines;
}
-function gatherCustomerData(lines, aCustomer) {}
-
-module.exports = { reportLines, gatherCustomerData };
+module.exports = { reportLines };
And we are done.
See below a chronology (from top to bottom) of all the refactoring steps:
Commit SHA | Message |
---|---|
8c61f3f | rename gatherCustomerData first parameter |
ffdf99a | move aggregation of the name field at gatherCustomerData into the caller |
27e61c8 | move aggregation of the location field at gatherCustomerData into the caller |
f6464dc | delete gatherCustomerData function |
The full commit history can be seen in the Commit History tab.