Daily Refactoring #1 - Extract Method

Overview

Extract method is probably the refactoring I use most frequently. It is covered in detail in The Refactoring Book, so I’ll leave out many of the details here that the book covers. I strongly encourage anyone interested in refactoring to read this book and keep it close by as a reference. The intent of this post and of this series of daily refactorings is to introduce a set of commonly used refactorings and to spark discussion about concrete real-world experience with them.

I want to get people thinking about the importance of refactoring and how it impacts how we think about upfront design vs just-in-time design and the ways in which refactoring can help us avoid over engineering.

What is Extract Method?

A refactoring that moves code out of a method by creating a new method and delegating to it.

To illustrate, let’s start with some code that could benefit from applying extract method:

public BigDecimal calculateTotal(List<item> items) {
 
	// calculate sub total
	BigDecimal subTotal = new BigDecimal("0.00");
	for (Item i : items) {
		subTotal.add(i.getAmount());
	}
 
	// calculate tax
	BigDecimal taxRate = TaxRateHelper.getTaxRateForState(_user.getState());
	BigDecimal taxAmount = subTotal.multiply(taxRate);
 
	BigDecimal total = taxRate + taxAmount;
	total.setScale(2, BigDecimal.ROUND_HALF_EVEN);
 
	return total;
}


This code isn’t terrible by any stretch but it could be cleaned up by applying extract method. It’s easy to see that more than one thing is being done in this method. It is calculating a sub-total, tax amount, and total. Let’s extract these out as separate methods:

public BigDecimal calculateTotal(List<item> items) {
	BigDecimal subTotal = calculateSubTotal(items);
	BigDecimal taxAmount = calculateTaxAmount(subTotal);
	BigDecimal total = taxRate + taxAmount;
	total.setScale(2, BigDecimal.ROUND_HALF_EVEN);
	return total;
}
 
public BigDecimal calculateSubTotal(List<item> items) {
	BigDecimal subTotal = new BigDecimal("0.00");
	for (Item i : items) {
		subTotal.add(i.getAmount());
	}
	return subTotal;
}
 
public BigDecimal calculateTaxAmount(BigDecimal amount) {
	BigDecimal taxRate = TaxRateHelper.getTaxRateForState(_customer.getState());
	BigDecimal taxAmount = subTotal.multiply(taxRate);
	return taxAmount;
}

What impact did this refactoring have? What were some of the benefits?

  • The original calculateTotal method is more cohesive and sticks to doing one thing.
  • The comments that described what the different sections of code were doing are no longer required because the method name clearly documents what the code does.
  • Each of the methods calculateTaxAmount and calculateSubTotal are no longer tightly coupled to calculateTotal and can be called independently.

Of all the benefits provided perhaps the one that I think has the most impact is that extract method increases cohesiveness and decreases coupling. The benefits of having a loosely coupled, highly cohesive system are well-worth the effort it takes to perform extract method, especially considering most modern development tools (such as ReSharper, IntelliJ, and Eclipse) support this refactoring.

Can you see any other refactorings that this code could benefit from?