/inline-function-refactoring

Working example with detailed commit history on the "inline function" refactoring from the Refactoring book by Fowler

Primary LanguageJavaScript

Continuous Integration

ℹ️ This repository is part of my "refactoring" catalog based on Fowler's book with the same title. Please see kaiosilveira/refactoring for more details.


Inline Function

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.

Working examples

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.

Working example #1: The getRating function

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.

Test suite

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);
  });
});

Steps

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 };

Commit history

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.

Working example #2: The reportLines function

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]);
}

Test suite

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);
  });
});

Steps

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.

Commit history

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.