Detailed reference for the refactorings that break down long methods into well-named, cohesive pieces. These are the most frequently used refactorings and the foundation of all code improvement.
The single most important refactoring. Turn a code fragment into a method whose name explains the purpose of the fragment.
You have a code fragment that can be grouped together. The longer a method, the harder it is to understand. When you see a comment explaining what the next block does, that's a signal to extract. The comment becomes the method name.
- Create a new method and name it after the intention of the code (what it does, not how)
- Copy the extracted code into the new method
- Scan the extracted code for references to local variables -- these become parameters or return values
- Declare any local-scope variables as local to the new method if they're only used within the extracted code
- Pass any remaining local variables as parameters
- If more than one value needs to be returned, consider returning an object or extracting further
- Replace the original code with a call to the new method
- Run tests
Before:
function printOwing(invoice) {
let outstanding = 0;
// print banner
console.log("***********************");
console.log("**** Customer Owes ****");
console.log("***********************");
// calculate outstanding
for (const order of invoice.orders) {
outstanding += order.amount;
}
// print details
console.log(`name: ${invoice.customer}`);
console.log(`amount: ${outstanding}`);
console.log(`due: ${invoice.dueDate.toLocaleDateString()}`);
}After:
function printOwing(invoice) {
printBanner();
const outstanding = calculateOutstanding(invoice);
printDetails(invoice, outstanding);
}
function printBanner() {
console.log("***********************");
console.log("**** Customer Owes ****");
console.log("***********************");
}
function calculateOutstanding(invoice) {
let result = 0;
for (const order of invoice.orders) {
result += order.amount;
}
return result;
}
function printDetails(invoice, outstanding) {
console.log(`name: ${invoice.customer}`);
console.log(`amount: ${outstanding}`);
console.log(`due: ${invoice.dueDate.toLocaleDateString()}`);
}| Bad Name | Good Name | Why |
|---|---|---|
doStuff() |
calculateMonthlyTotal() |
Names the intent, not the vagueness |
process() |
validateAndSaveOrder() |
Specific about what it does |
handleData() |
parseCSVRow() |
Names the domain concept |
helper() |
formatCurrencyForDisplay() |
Describes the transformation |
step2() |
applyDiscountRules() |
Names the business concept |
Rule of thumb: If you can't find a good name, the extraction boundaries may be wrong. Try extracting a different fragment.
The inverse of Extract Method. Replace a method call with the method's body when the body is as clear as the name, or when you need to regroup poorly factored code.
Sometimes a method body is as obvious as the method name. Indirection without value is noise. Also useful as an intermediate step: inline a badly decomposed method, then re-extract along better boundaries.
- Check that the method is not polymorphic (no subclass overrides it)
- Find all callers
- Replace each call with the method body
- Delete the method
- Run tests
Before:
def get_rating(self):
return 2 if self.more_than_five_late_deliveries() else 1
def more_than_five_late_deliveries(self):
return self.late_deliveries > 5After:
def get_rating(self):
return 2 if self.late_deliveries > 5 else 1- When the method name communicates domain meaning the code doesn't
- When the method is used in multiple places (DRY)
- When the method is overridden in subclasses
Introduce a local variable for a complex expression to make it self-documenting.
Expressions can become hard to read. A well-named variable for a sub-expression acts as inline documentation and makes debugging easier.
- Identify a complex expression or sub-expression
- Declare a variable named for the intent of the expression
- Replace the expression with the variable
- Run tests
Before:
return order.quantity * order.itemPrice -
Math.max(0, order.quantity - 500) * order.itemPrice * 0.05 +
Math.min(order.quantity * order.itemPrice * 0.1, 100);After:
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 500) * order.itemPrice * 0.05;
const shippingCap = Math.min(basePrice * 0.1, 100);
return basePrice - quantityDiscount + shippingCap;The inverse of Extract Variable. Remove a variable when the expression is just as clear.
- The variable name adds no information beyond what the expression says
- The variable is assigned once and used once
- The variable is blocking another refactoring (e.g., you need to inline it to then Extract Method)
Before:
base_price = order.base_price()
return base_price > 1000After:
return order.base_price() > 1000Turn a temporary variable into a method call so the computation is reusable and the original method becomes shorter.
Temporaries can only be seen within a single method. If the same computation is needed elsewhere, it gets duplicated. A query method is visible to the whole class (or can be extracted to another class).
- Check that the variable is assigned once and the expression has no side effects
- Extract the right-hand side of the assignment into a new method
- Replace all references to the temp with calls to the new method
- Remove the temp declaration and assignment
- Run tests
Before:
class Order {
getPrice() {
const basePrice = this.quantity * this.itemPrice;
if (basePrice > 1000) {
return basePrice * 0.95;
} else {
return basePrice * 0.98;
}
}
}After:
class Order {
getPrice() {
if (this.basePrice() > 1000) {
return this.basePrice() * 0.95;
} else {
return this.basePrice() * 0.98;
}
}
basePrice() {
return this.quantity * this.itemPrice;
}
}Calling the method multiple times instead of caching in a temp may seem wasteful. In practice, the performance impact is negligible for most code. Profile before optimizing. Refactored code is easier to optimize later because the hot path is isolated.
When a temporary variable is assigned more than once (and it's not a loop counter or collecting variable), it's doing two different jobs. Give each job its own variable.
A temp assigned twice for different purposes misleads the reader into thinking the assignments are related. Each role deserves its own variable with a descriptive name.
- Rename the first assignment to reflect its purpose
- Declare it as
const/finalif possible - Find all uses that refer to the first assignment's value and make sure they use the new name
- Repeat for each subsequent assignment with a different name
- Run tests
Before:
let temp = 2 * (height + width); // perimeter
console.log(temp);
temp = height * width; // area
console.log(temp);After:
const perimeter = 2 * (height + width);
console.log(perimeter);
const area = height * width;
console.log(area);Never assign to a parameter inside a method body. It confuses readers about whether the change is visible to the caller (it isn't in pass-by-value languages; it is in pass-by-reference for object mutations).
- Create a new local variable for the parameter
- Replace all assignments to the parameter with assignments to the new variable
- Run tests
Before:
def discount(input_val, quantity):
if quantity > 50:
input_val -= 2
if quantity > 100:
input_val -= 1
return input_valAfter:
def discount(input_val, quantity):
result = input_val
if quantity > 50:
result -= 2
if quantity > 100:
result -= 1
return resultWhen a method is too tangled with local variables to extract from, move the entire method into its own class where the local variables become fields. Then you can freely extract sub-methods.
Sometimes a long method has so many interrelated local variables that Extract Method is impossible (too many parameters would be needed). By turning the method into its own class, all locals become fields, accessible to any extracted method without parameters.
- Create a new class named after the method's purpose
- Add a field for the original object and for every local variable and parameter
- Create a constructor that takes the original object and all parameters
- Copy the method body into a
compute()(or similar) method - Replace the original method with: create the new object, call
compute() - Now freely extract methods within the new class -- locals are fields, no parameter passing needed
- Run tests
Before:
class Account:
def gamma(self, input_val, quantity, year_to_date):
# 50 lines of tangled computation using all three params
# plus self.fields -- too intertwined to extract
...After:
class GammaCalculation:
def __init__(self, account, input_val, quantity, year_to_date):
self.account = account
self.input_val = input_val
self.quantity = quantity
self.year_to_date = year_to_date
def compute(self):
# Now extract freely -- all variables are fields
self._apply_quantity_adjustment()
self._apply_yearly_factor()
return self.input_val
def _apply_quantity_adjustment(self):
# can access self.quantity, self.input_val freely
...
def _apply_yearly_factor(self):
...
class Account:
def gamma(self, input_val, quantity, year_to_date):
return GammaCalculation(self, input_val, quantity, year_to_date).compute()| Situation | Refactoring |
|---|---|
| Code block can be named by intent | Extract Method |
| Method body is trivial and name adds nothing | Inline Method |
| Complex expression needs explanation | Extract Variable |
| Variable adds no meaning beyond the expression | Inline Variable |
| Same computation needed in multiple methods | Replace Temp with Query |
| One variable serves two purposes | Split Temporary Variable |
| Parameter is reassigned inside method | Remove Assignments to Parameters |
| Long method with too many entangled locals | Replace Method with Method Object |