ℹ️ 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: Introduce Explaining Variable
Before | After |
---|---|
return (
order.quantity * order.itemPrice -
Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
Math.min(order.quantity * order.itemPrice * 0.1, 100);
); |
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
const shipping = Math.min(basePrice * 0.1, 100);
return basePrice - quantityDiscount + shipping; |
Inverse of: Inline Variable
Oftentimes, expressions become hard to read and add a lot of overhead when trying to understand a given piece of code. Extracting a variable for a complex expression helps the reader by reducing the amount of logic to interpret, replacing it basically with well-structured text.
For this refactoring, we are going to analyze two working examples that calculate the final price of an order: a functional approach and an Object-Oriented approach.
As our first working example, we start with a code to calculate a price
, based on some discount rules. The initial code looks like this:
function price(order) {
// price is base price - quantity discount + shipping
return (
order.quantity * order.itemPrice -
Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
Math.min(order.quantity * order.itemPrice * 0.1, 100)
);
}
The unit testing approach for this example was tricky, mainly because the price
function is doing multiple things. Tests were added to cover the following aspects of it:
- Customers should have a 5% discount per item for every item, starting after 500 units
- Customers should be charged either 10% of the total base price of the order or 100, whatever amount is the lowest.
For implementation details, please see function.test.js.
To perform this refactoring we start by introducing a new variable called basePrice
, which will not be used yet:
diff --git a/function.js b/function.js
@@ -1,5 +1,6 @@
function price(order) {
// price is base price - quantity discount + shipping
+ const basePrice = order.quantity * order.itemPrice;
return (
order.quantity * order.itemPrice -
Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
Then, we replace the occurrence of its value in the two places it occurs:
diff --git a/function.js b/function.js
@@ -2,9 +2,9 @@ function price(order) {
// price is base price - quantity discount + shipping
const basePrice = order.quantity * order.itemPrice;
return (
- order.quantity * order.itemPrice -
+ basePrice -
Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
- Math.min(order.quantity * order.itemPrice * 0.1, 100)
+ Math.min(basePrice * 0.1, 100)
);
}
Then, we move on to the discount part:
diff --git a/function.js b/function.js
@@ -1,6 +1,7 @@
function price(order) {
// price is base price - quantity discount + shipping
const basePrice = order.quantity * order.itemPrice;
+ const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
return (
basePrice -
Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
Then, we replace the discount expression by its already calculated value:
diff --git a/function.js b/function.js
@@ -2,11 +2,7 @@ function price(order) {
// price is base price - quantity discount + shipping
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
- return (
- basePrice -
- Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
- Math.min(basePrice * 0.1, 100)
- );
+ return basePrice - quantityDiscount + Math.min(basePrice * 0.1, 100);
}
module.exports = { price };
And the same applies to the shipping calculation. First, we introduce the shipping variable:
diff --git a/function.js b/function.js
@@ -2,7 +2,8 @@ function price(order) {
// price is base price - quantity discount + shipping
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
- return basePrice - quantityDiscount + Math.min(basePrice * 0.1, 100);
+ const shipping = Math.min(basePrice * 0.1, 100);
+ return basePrice - quantityDiscount + shipping;
}
module.exports = { price };
Then we can replace the shipping calculation with its variable:
diff --git a/function.js b/function.js
@@ -3,7 +3,7 @@ function price(order) {
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
const shipping = quantityDiscount + Math.min(basePrice * 0.1, 100);
- return basePrice - quantityDiscount + Math.min(basePrice * 0.1, 100);
+ return basePrice - quantityDiscount + shipping;
}
module.exports = { price };
And, finally, we can remove the useless comment:
diff --git a/function.js b/function.js
@@ -1,5 +1,4 @@
function price(order) {
- // price is base price - quantity discount + shipping
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
const shipping = quantityDiscount + Math.min(basePrice * 0.1, 100);
See below a chronology (from top to bottom) of all the refactoring steps:
Commit SHA | Message |
---|---|
7428c73 | introduce the basePrice variable |
b36df25 | replace base price calculation by its new variable |
576ee4b | introduce quantityDiscount |
a196cf5 | replace discount expression by its variable |
f08c850 | introduce the shipping variable |
446ff41 | remove now useless comment |
The full commit history can be seen in the Commit History tab.
Our second working example is an Order
class that has a getter to provide its price
. The code in the price
getter is complicated and full of math expressions without much reasoning.
class Order {
constructor(aRecord) {
this._data = aRecord;
}
get quantity() {
return this._data.quantity;
}
get itemPrice() {
return this._data.itemPrice;
}
get price() {
return (
this.quantity * this.itemPrice -
Math.max(0, this.quantity - 500) * this.itemPrice * 0.05 +
Math.min(this.quantity * this.itemPrice * 0.1, 100)
);
}
}
Unit testing for this object-oriented approach is more straightforward, as we can simply adapt the same tests created for the functional example. On top of that, some other unit tests were added to perform some simple assertions on the getters (quantity
and itemPrice
), though, to make sure they're matching what's being passed as the source of truth, i.e., the record
data.
For implementation details, see order.test.js.
For this example, we have an Order
class that holds the data for a given record
and knows how to calculate its final price. The code inside the price
method is actually the same one used in the functional example. To refactor that, we:
- introduce
basePrice
getter in the Order class:
diff --git a/order.js b/order.js
@@ -11,6 +11,10 @@ class Order {
return this._data.itemPrice;
}
+ get basePrice() {
+ return this.quantity * this.itemPrice;
+ }
+
get price() {
return (
this.quantity * this.itemPrice -
- use the encapsulated
basePrice
getter atOrder.price
:
diff --git a/order.js b/order.js
@@ -17,7 +17,7 @@ class Order {
get price() {
return (
- this.quantity * this.itemPrice -
+ this.basePrice -
Math.max(0, this.quantity - 500) * this.itemPrice * 0.05 +
Math.min(this.quantity * this.itemPrice * 0.1, 100)
);
- introduce
quantityDiscount
getter:
diff --git a/order.js b/order.js
@@ -15,6 +15,10 @@ class Order {
return this.quantity * this.itemPrice;
}
+ get quantityDiscount() {
+ return Math.max(0, this.quantity - 500) * this.itemPrice * 0.05;
+ }
+
get price() {
return (
this.basePrice -
- use encapsulated
quantityDiscount
getter atOrder.price
:
diff --git a/order.js b/order.js
@@ -21,9 +21,7 @@ class Order {
get price() {
return (
- this.basePrice -
- Math.max(0, this.quantity - 500) * this.itemPrice * 0.05 +
- Math.min(this.quantity * this.itemPrice * 0.1, 100)
+ this.basePrice - this.quantityDiscount + Math.min(this.quantity * this.itemPrice * 0.1, 100)
);
}
}
- introduce
shipping
getter:
diff --git a/order.js b/order.js
@@ -19,6 +19,10 @@ class Order {
return Math.max(0, this.quantity - 500) * this.itemPrice * 0.05;
}
+ get shipping() {
+ return Math.min(this.quantity * this.itemPrice * 0.1, 100);
+ }
+
get price() {
return (
this.basePrice - this.quantityDiscount + Math.min(this.quantity * this.itemPrice * 0.1, 100)
- use encapsulated
shipping
getter atOrder.price
:
diff --git a/order.js b/order.js
@@ -24,9 +24,7 @@ class Order {
}
get price() {
- return (
- this.basePrice - this.quantityDiscount + Math.min(this.quantity * this.itemPrice * 0.1, 100)
- );
+ return this.basePrice - this.quantityDiscount + this.shipping;
}
}
And we are done!
See below a chronology (from top to bottom) of all the refactoring steps:
Commit SHA | Message |
---|---|
b01ce19 | introduce basePrice getter in the Order class |
50242dc | use the encapsulated basePrice getter at Order.price |
725105d | introduce quantityDiscount getter |
9844be3 | use encapsulated shipping getter at Order.price |
02cf861 | introduce shipping getter |
bf684fd | use encapsulated quantityDiscount getter at Order.price |
The full commit history can be seen in the Commit History tab.