In this post I would like to show you “Refactoring Pyramid in Practice” to clean your code. In order to do so, I will follow the sequence of small transformations like in one of refactoring examples in “Refactoring to Patterns” book by Joshua Kerievsky. The sample will show refactoring to Interpreter. Anyway I’ve experienced the same refactoring flow when following my “Pyramid of refactoring” concept. Therefore I think it can be applied to majority of examples from this book, which is a proof that the concept is useful in lots of other cases.
In my previous post How Pyramid of Refactoring was disovered? I talked about origins of Pyramid of Refactoring. Something that inspires me, something I try to engage in lots of places / techniques like Test Driven Development, working with with Legacy Code or just when our daily programming activities take places.
Nowadays applying refactoring techniques is much easier, as tools like (especially) IntelliJ but also Eclipse, NetBeans provide us with lots of automated code transformations. Moreover when we use these transformations wisely then the logic of our code should still work as expected (or rather as it worked before…). Some exceptions to this rule are transformations like “extract field” because when a method sets up a field instead of using local variables we might run into concurrency issues when second thread is running the same method at the same time.
Let’s start with extract from ProductFinder class.
public class ProductFinder {
private List<Product> repository;
public ProductFinder(List<Product> repository) {
this.repository = repository;
}
public List<Product> byBelowPrice(float price) {
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (product.getPrice() < price)
foundProducts.add(product);
}
return foundProducts;
}
public List<Product> byColor(ProductColor color) {
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (product.getColor().equals(color))
foundProducts.add(product);
}
return foundProducts;
}
}
As you see both of the methods are almost the same. The only difference is the condition to to check if given Product should be considered as part of valid output or not.
Level of methods
Let’s extract the difference (one of the conditions) into a separate method for a while.
public class ProductFinder {
private List<Product> repository;
public ProductFinder(List<Product> repository) {
this.repository = repository;
}
public List<Product> byBelowPrice(float price) {
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (isSatisfiedBy(price, product))
foundProducts.add(product);
}
return foundProducts;
}
private boolean isSatisfiedBy(float price, Product product) {
return product.getPrice() < price;
}
}
Now when you have a closer look at the extracted method you notice that calling byBelowPrice method entails calling isSatisfiedBy method. But in case of 1000 products the isSatisfiedBy method is called 1000 times with the same value of first parameter (price) and different value of the second parameter (Product).
Such a notice leans our thoughts towards creating BelowPriceSpec class that holds a field that stores the price, and contains a method isSatisfiedBy that takes a single parameter Product.
Level of classes
public class BelowPriceSpec {
private float price;
public BelowPriceSpec(float price) {
this.price = price;
}
public float getPrice() {
return price;
}
}
public class ProductFinder {
...
public List<Product> byBelowPrice(float price) {
BelowPriceSpec belowPriceSpec = new BelowPriceSpec(price);
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (product.getPrice() < belowPriceSpec.getPrice())
foundProducts.add(product);
}
return foundProducts;
}
}
Please note that currently the condition uses “belowPriceSpec.getPrice()” instead of parameter price directly. This way when I extract isSatisfiedBy method method again its first parameter is different. Next I can move this method into BelowPriceSpec class.
public class ProductFinder {
…
public List<Product> byBelowPrice(float price) {
BelowPriceSpec belowPriceSpec = new BelowPriceSpec(price);
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (isSatisfiedBy(belowPriceSpec, product))
foundProducts.add(product);
}
return foundProducts;
}
private boolean isSatisfiedBy(BelowPriceSpec belowPriceSpec, Product product) {
return product.getPrice() < belowPriceSpec.getPrice();
}
}
Level of abstractions / patterns
Here is how BelowPriceSpec class looks like after isSatisfiedBy method has been moved into it. I have already inlined the only temporary getter (getPrice).
public class BelowPriceSpec {
private float price;
public BelowPriceSpec(float price) {
this.price = price;
}
public oolean isSatisfiedBy(Product product) {
return product.getPrice() < price;
}
}
Our next thoughts say that we might end up with lots of such classes. They will be DeliverySpec, SizeSpec, MinAgeSpec (if it is a toy) and so on. This classes will be married by our Spec interfaces, so we can extract it.
public interface Spec {
boolean isSatisfiedBy(Product product);
}
public class BelowPriceSpec implements Spec {
private float price;
public BelowPriceSpec(float price) {
this.price = price;
}
@Override
public boolean isSatisfiedBy(Product product) {
return product.getPrice() < price;
}
}
public class ProductFinder {
private List<Product> repository;
public ProductFinder(List<Product> repository) {
this.repository = repository;
}
public List<Product> byBelowPrice(float price) {
Spec spec = new BelowPriceSpec(price);
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (spec.isSatisfiedBy(product))
foundProducts.add(product);
}
return foundProducts;
}
}
Coming back to Method Level
Now the interfaces has been extracted and is used within byBelowPrice method. But majority of this method is using the Spec interface, not BelowPriceSpec. This way we might extract the part of methods that would be common between similar methods (like byMinAge, bySize). We follow by extracting a brand new method bySpec(Spec spec).
public class ProductFinder {
private List<Product> repository;
public ProductFinder(List<Product> repository) {
this.repository = repository;
}
private List<Product> bySpec(Spec spec) {
List<Product> foundProducts = new ArrayList<>();
Iterator<Product> products = repository.iterator();
while (products.hasNext()) {
Product product = products.next();
if (spec.isSatisfiedBy(product))
foundProducts.add(product);
}
return foundProducts;
}
public List<Product> byBelowPrice(float price) {
Spec spec = new BelowPriceSpec(price);
return bySpec(spec);
}
}
Flow Level
Now we are going to have a closer look at bySpec method. It uses very old constructions coming from Java pre 1.5. Let change it into for loop and streams. Martin Fowler in his last book put additional refactoring called “Replace for loop with a stream”.
public class ProductFinder {
private List<Product> repository;
public ProductFinder(List<Product> repository) {
this.repository = repository;
}
public List<Product> bySpec(Spec spec) {
return repository.stream()
.filter(spec::isSatisfiedBy)
.collect(Collectors.toList());
}
@Deprecated
public List<Product> byBelowPrice(float price) {
return bySpec(new BelowPriceSpec(price));
}
}
The other method byBelowPrice does not require “spec” local variable for its readability so it has been inlined.
Architecture Level
Finally we might think about improving Architectures. The current article is growing so I have decided to explain Architectures level as part of next article. How SOLID Principles support refactoring?
