I recently browsed again the excellent book Code Complete from Steve McConnell while in the middle of several Java code reviews. Although the book is nor specifically dedicated to Java code, I still found it quite useful and inspiring.
The chapter on refactoring in particular offers a very practical perspective that all programmer should have in mind when it comes to infuse a dose of evolution in the life cycle of their software.
What I found particularly valuable are the checklists that the author has put together: reasons to refactor, specific re-factorings (data level, statement level, routine-level, class-implementation, class-interface, system-level), refactoring safely, strategies, summary.
In this post, I would like to illustrate some of these refactoring techniques with java code snippets.
Since I do not have time to comment every item in these checklists, I decided to pick-up those that are not trivial or obvious and might bring the best return on your refactoring investment. These items are high-lighted in green and developed in sections at the end of this article.
If you think that missing items are worth explaining, please add a comment to this post and I will be glad to add a specific paragraph for those as well.
Checklist: Reasons to Refactor
- Code is duplicate
- A routine is too long
- A loop is too long or too deeply nested
- A class/interface/method has poor cohesion ⇐
- A class interface does not provide a consistent level of abstraction ⇐
- A parameter list has too many parameters ⇐
- Changes within a class tend to be compartmentalized
- Changes require parallel modifications to multiple classes ⇐
- Inheritance hierarchies have to be modified in parallel
- Related data items that are used together are not organized into classes ⇐
- A routine uses more features of another class than of its own class
- A primitive data type is overloaded
- A class doesn't do very much
- A chain of routines passes tramp data ⇐
- A middle man object isn't doing anything
- One class is overly intimate with another
- A routine has a poor name
- Data members are public
- A subclass uses only a small percentage of its parents' routines
- Comments are used to explain difficult code
- Global variables are used
- A routine uses setup code before a routine call or takedown code after a routine call
- A program contains code that seems like it might be needed someday
Checklist: Summary of Refactorings
Data Level Refactoring
- Replace a magic number with a named constant ⇐
- Rename a variable with a clearer or more informative name
- Move an expression inline
- Replace an expression with a routine
- Introduce an intermediate variable
- Convert a multi-use variable to a multiple single-use variables ⇐
- Use a local variable for local purposes rather than a parameter
- Convert a data primitive to a class
- Convert a set of type codes to a class
- Convert a set of type codes to a class with subclasses
- Change an array to an object
- Encapsulate a collection ⇐
- Replace a traditional record with a data class
Statement Level Refactorings
- Decompose a boolean expression
- Move a complex boolean expression into a well-named boolean function
- Consolidate fragments that are duplicated within different parts of a conditional
- Use break or return instead of a loop control variable
- Return as soon as you know the answer instead of assigning a return value within nested if-then-else statements
- Replace conditionals with polymorphism (especially repeated case statements) ⇐
- Create and use null objects instead of testing for null values
Routine Level Refactorings
- Extract a routine
- Move a routine's code inline
- Convert a long routine to a class
- Substitute a simple algorithm for a complex algorithm
- Add a parameter
- Remove a parameter
- Separate query operations from modification operations
- Combine similar routines by parameterizing them
- Separate routines whose behavior depends on parameters passed in ⇐
- Pass a whole object rather than specific fields
- Pass specific fields rather than a whole object
- Encapsulate downcasting ⇐
Class Implementation Refactorings
- Change value objects to reference objects
- Change reference objects to value objects
- Replace virtual routines with data initialization
- Change member routine or data placement
- Extract specialized code into a subclass ⇐
- Combine similar code into a superclass
Class Interface Refactorings
- Move a routine to another class
- Convert one class to two
- Eliminate a class
- Hide a delegate ⇐
- Replace inheritance with delegation
- Replace delegation with inheritance
- Remove a middle man ⇐
- Introduce a foreign routine
- Introduce a class extension
- Encapsulate an exposed member variable
- Remove Set() routines for fields that cannot be changed
- Hide routines that are not intended to be used outside the class
- Encapsulate unused routines ⇐
- Collapse a superclass and subclass if their implementations are very similar
System Level Refactorings
- Duplicate data you can't control ⇐
- Change unidirectional class association to bidirectional class association
- Change bidirectional class association to unidirectional class association
- Provide a factory routine rather than a simple constructor
- Replace error codes with exceptions or vice versa
Selected items refactoring samples in java
- A class/interface/method has poor cohesion
- A class, interface or method should be only responsible for a precise and meaningful set of functionality and behavior. For example,the interface KitchenAppliance below lacks cohesion. A generic kitchen appliance should not allow to wash both clothes and dishes.
public interface KitchenAppliance {
public float waterConsumption = 0;
public float energyConsumption = 0;
void washClothes();
void washDishes();
}
⇒
public interface WashingKitchenAppliance {
public float waterConsumption = 0;
public float energyConsumption = 0;
void wash();
}
- In Java, a consistent level of abstraction is usually obtained when the objects that represent abstract actors perform work, report on, change their state and communicate with other java objects in a a compatible and comparable way.
The use of inheritance, encapsulation and polymorphism is usually the key to a consistent level of abstraction and is at the core of good design patterns. In the example below, an intermediary level of abstraction, e.g. DairyAnimal (cows, goats) and MeatAnimal (pigs, steers), could be created to avoid specifying the type of food for each animal and focus on more specific features for the sub-classes.
public class Animal extends LivingThing
{
private Location loc;
private double energyReserves;
public boolean isHungry() {
return energyReserves < 2.5;
}
public void eat(Food food) {
energyReserves += food.getCalories();
}
public void moveTo(Location location) {
this.loc = location;
}
}
thePig = new Animal();
theCow = new Animal();
if (thePig.isHungry()) {
thePig.eat(tableScraps);
}
if (theCow.isHungry()) {
theCow.eat(grass);
}
theCow.moveTo(theBarn);
⇒
public class DairyAnimal extends Animal {
private Double milkProduction;
public Double getMilkProduction() {
return milkProduction;
}
...
}
- Methods with too many parameters is an indication that the code has either been badly designed initially (lack of abstraction) or has evolved over the months or years without proper refactoring. One elegant solution for creational methods/constructor requiring a large number of parameters is to use a builder pattern well define in Joshua Bloch's Effective Java and offers a way to use optional parameters:
public NutritionFacts(int servingSize, int servings, int sodium, int iron, int fat, int carbs) {
this.servingSize = servingSize;
this.sodium = sodium;
this.servingSize = servingSize;
this.iron = iron;
this.fat = fat;
this.carbs = carbs;
}
NutritionFacts soda = new NutritionFacts(240, 8, 35, 13, 0, 27);
⇒
public class NutritionFacts {
private int servingSize = 0;
private int servings = 0;
private int sodium = 0;
private int iron = 0;
private int fat = 0;
private int carbs = 0;
public static class Builder {
private int servingSize;
private int servings;
private int sodium;
private int iron;
private int fat;
private int carbs;
public Builder(int servingSize, int servings) {
this.servingSize = servingSize;
this.servings = servings;
}
public Builder sodium(int val) {
this.sodium = val;
return this;
}
public Builder iron(int val) {
this.iron = val;
return this;
}
public Builder fat(int val) {
this.fat = val;
return this;
}
public Builder carbs(int val) {
this.carbs = val;
return this;
}
public NutritionFacts build() {
return new NutritionFacts(this);
}
}
private NutritionFacts(Builder builder) {
servingSize = builder.servingSize;
servings = builder.servings;
sodium = builder.sodium;
iron = builder.iron;
fat = builder.fat;
carbs = builder.carbs;
}
}
NutritionFacts soda = new NutritionFacts(240, 8).build();
or
NutritionFacts soda = new NutritionFacts(240, 8).sodium(35).iron(13).fat(0).carbs(27).build();
or
NutritionFacts soda = new NutritionFacts(240, 8).sodium(35).iron(13).carbs(27).build();
- If you regularly have to modify the same set of classes then try to refactor these classes with the correct level of abstraction and factorization so changes only affect one class.
- Recurrent set of operations can be combined in its own class:
theCow = new DairyAnimal();
theCow.brush();
theCow.feed();
theCow.milk();
theCow.feed();
⇒
public class DairyAnimalOperation {
public void tend(DairyAnimal dairyAnimal) {
dairyAnimal.brush();
dairyAnimal.feed();
dairyAnimal.milk();
dairyAnimal.feed();
};
Even better, refactoring can include the use of a flexible strategy pattern, if the recurrent process needs to be dynamically.
- "Tramp data" refers to passing data to one method so it can be passed to another one (Page-Jones 1988). This can be solved by introducing a better level of abstraction in the interfaces in question
- The word magic refers to the use of numeric or string literal. Use constants or Enums instead.
public double areaOfCircle (double radius) {
return Math.pow(radius * 3.14159, 2);
}
⇒
private static final double PI = 3.14159;
public double areaOfCircle (double radius) {
return Math.pow(radius * PI, 2);
}
- Common offenders are variable names like i,j, temp. These need to be replaced by specific names (e.g. row, column).
for (int i=0;i&tl;N;i++) {
for (int j=0;j&tl;M;j++) {
matrix[i][j] = 0;
}
}
⇒
for (int row=0;row&tl;numRows;row++) {
for (int colum=0;colum&tl;numColumns;colum++) {
matrix[row][colum] = 0;
}
}
- Instead of returning the collection itself, encapsulate a read-only collection instance with specific methods to add or remove elements in the original collection. You can leverage unmodifiable view methods that are part of the collection class for this:
public static <T> Collection<T> unmodifiableCollection(Collection<? extends T> c)
public static <T> Set<T> unmodifiableSet(Set<? extends T> s)
public static <T> SortedSet<T> unmodifiableSortedSet(SortedSet<T> s)
public static <T> List<T> unmodifiableList(List<? extends T> list)
public static <K,V> Map<K,V> unmodifiableMap(Map<? extends K,? extends V> m)
public static <K,V> SortedMap<K,V> unmodifiableSortedMap(SortedMap<K,? extends V> m)
- With polymorphism, subclasses of a class possesses their own specific behaviors and share also some of the functionality of their parent class. Refactor conditionals, especially complex case statements with polymorphism design:
Bike myBike = new Bike();
String bikeType = "default";
witch(myBike.getTireWidth()) {
case 23 :
if (myBike.getSuspension().compareToIgnoreCase("Simple") == 0) {
bikeType = "Road bike";
}
break;
default :
if (myBike.getSuspension().compareToIgnoreCase("Dual") == 0) {
bikeType = "Mountain bike";
}
break;
}
⇒
public class MountainBike extends Bicycle {
private String suspension;
public MountainBike(
int startCadence,
int startSpeed,
int startGear,
String suspensionType){
super(startCadence,
startSpeed,
startGear);
this.setSuspension(suspensionType);
}
public String getSuspension(){
return this.suspension;
}
public void setSuspension(String suspensionType) {
this.suspension = suspensionType;
}
}
public class RoadBike extends Bicycle{
private int tireWidth;
public RoadBike(int startCadence,
int startSpeed,
int startGear,
int newTireWidth){
super(startCadence,
startSpeed,
startGear);
this.setTireWidth(newTireWidth);
}
public int getTireWidth(){
return this.tireWidth;
}
public void setTireWidth(int newTireWidth){
this.tireWidth = newTireWidth;
}
}
- If you have a method which executes different statements based on a particular parameter value, figure out if you can break the method into separate ones without passing the parameter in question.
- Objects returned by a method should use the most specific type possible.
public class A {
public String doSomething() {
return "Do something.";
}
}
public class B extends A {
public String doSomethingMoreSpecific() {
return "Do something more specific.";
}
}
public class DownCast {
private List<A> list;
public List<A> getList() {
return list;
}
public List<B> getMostSpecificList() {
return (List<B>) (List<?>) list;
}
public void setList(List<A> list) {
this.list = list;
}
public DownCast() {
list = new ArrayList<A>();
for (int i=0;i<10;i++) {
list.add(new B());
}
}
}
public static void main(String[] args) {
DownCast obj = new DownCast();
List<A> listOfA = obj.getList();
System.out.println(listOfA.get(0).doSomething());
List<B> listOfB = obj.getMostSpecificList();
System.out.println(listOfB.get(0).doSomethingMoreSpecific());
}
- Refactor specialized code into specific own subclasses if the code in question is only used in a subset of initial classes.
- If Class A calls B then C while A was supposed to only call B and B calls C, then dissociates C from A and have B calls C only.
- On the other hand, if you think that A should call C directly as long as the role of B is still clearly defines, does not have overlap with C and can be used independently of C.
- Create new interfaces for the most common used set of methods
public class Bird {
private String species;
private String eggColor;
private String food;
private int flySpeed;
private int flyHeight;
private String dragType;
private String wingType;
public String getSpecies() {
return species;
}
public void setSpecies(String species) {
this.species = species;
}
public String getEggColor() {
return eggColor;
}
public void setEggColor(String eggColor) {
this.eggColor = eggColor;
}
public String getFood() {
return food;
}
public void setFood(String food) {
this.food = food;
}
public int getFlySpeed() {
return flySpeed;
}
public void setFlySpeed(int flySpeed) {
this.flySpeed = flySpeed;
}
...
}
⇒
public interface BirdFlight {
public int getFlySpeed();
public void setFlySpeed(int flySpeed);
public int getFlyHeight();
public void setFlyHeight(int flyHeight);
public String getDragType();
public void setDragType(String dragType);
public String getWingType() ;
public void setWingType(String wingType);
}
public class Bird implements BirdFlight {
private String species;
private String eggColor;
private String food;
public String getSpecies() {
return species;
}
public void setSpecies(String species) {
this.species = species;
}
public String getEggColor() {
return eggColor;
}
public void setEggColor(String eggColor) {
this.eggColor = eggColor;
}
public String getFood() {
return food;
}
public void setFood(String food) {
this.food = food;
}
@Override
public int getFlySpeed() {
...
}
@Override
public void setFlySpeed(int flySpeed) {
...
}
@Override
public int getFlyHeight() {
...
}
@Override
public void setFlyHeight(int flyHeight) {
...
}
...
}
- As long as layering principles are not violated, wrap or access data maintain by the system via a consistent way/interface/API. A typical examples is data shares in GUI control in the front-end layer. Try to find a way to copy/mirror the data in question and treat it as reference source of data.
