Java Code Refactor Methods
The main goal of this artcile is to illustrate the difference between dirty and clean code, and teach you how to convert one into another. We will start by learning the different code smells — examples of how you shouldn’t write and structure the code. Then we will go over a broad range of refactoring techniques and learn how to apply them to improve ugly code.
Code Smells
Code smells are key signs that refactoring is necessary. In the process of refactoring, we get rid of smells, enabling further development of the application with equal or greater speed.
The lack of regular refactoring, can lead to a complete paralysis of a project over time, wasting a few years of development and requiring you to spend several more years to rewrite it from scratch.
Therefore, it is necessary to get rid of code smells while they are still small.
Bloaters
Bloaters are code, methods and classes that have increased to such gargantuan proportions that they are hard to work with. Usually these smells do not crop up right away, rather they accumulate over time as the program evolves (and especially when nobody makes an effort to eradicate them).
Long Method
A method contains too many lines of code. Generally, any method longer than ten lines should make you start asking questions.
Reasons for the Problem
Like the Hotel California, something is always being added to a method but nothing is ever taken out. Since it’s easier to write code than to read it, this “smell” remains unnoticed until the method turns into an ugly, oversized beast.
Mentally, it’s often harder to create a new method than to add to an existing one: “But it’s just two lines, there’s no use in creating a whole method just for that…” Which means that another line is added and then yet another, giving birth to a tangle of spaghetti code.
Treatment
As a rule of thumb, if you feel the need to comment on something inside a method, you should take this code and put it in a new method. Even a single line can and should be split off into a separate method, if it requires explanations. And if the method has a descriptive name, nobody will need to look at the code to see what it does.
Extract Method
To reduce the length of a method body, use Extract Method.
Problem
You have a code fragment that can be grouped together.
void printOwing() {
printBanner();
// Print details.
System.out.println("name: " + name);
System.out.println("amount: " + getOutstanding());
}
Solution
Move this code to a separate new method (or function) and replace the old code with a call to the method.
void printOwing() {
printBanner();
printDetails(getOutstanding());
}
void printDetails(double outstanding) {
System.out.println("name: " + name);
System.out.println("amount: " + outstanding);
}
Reduce Local Variables and Parameters Before Extracting a Method
If local variables and parameters interfere with extracting a method, use Replace Temp with Query, Introduce Parameter Object or Preserve Whole Object.
Problem
You place the result of an expression in a local variable for later use in your code.
double calculateTotal() {
double basePrice = quantity * itemPrice;
if (basePrice > 1000) {
return basePrice * 0.95;
}
else {
return basePrice * 0.98;
}
}
Solution
Move the entire expression to a separate method and return the result from it. Query the method instead of using a variable. Incorporate the new method in other methods, if necessar
double calculateTotal() {
if (basePrice() > 1000) {
return basePrice() * 0.95;
}
else {
return basePrice() * 0.98;
}
}
double basePrice() {
return quantity * itemPrice;
}
Introduce Parameter Object
If local variables and parameters interfere with extracting a method, use Replace Temp with Query, Introduce Parameter Object or Preserve Whole Object.
Problem
Your methods contain a repeating group of parameters.
Solution
Replace these parameters with an object.
Preserve Whole Object
If local variables and parameters interfere with extracting a method, use Replace Temp with Query, Introduce Parameter Object or Preserve Whole Object.
Problem
You get several values from an object and then pass them as parameters to a method.
int low = daysTempRange.getLow();
int high = daysTempRange.getHigh();
boolean withinPlan = plan.withinRange(low, high);
Solution
Instead, try passing the whole object.
boolean withinPlan = plan.withinRange(daysTempRange);
Replace Method with Method Object
If none of the previous recipes help, try moving the entire method to a separate object via Replace Method with Method Object.
Problem
You have a long method in which the local variables are so intertwined that you can’t apply Extract Method.
class Order {
// ...
public double price() {
double primaryBasePrice;
double secondaryBasePrice;
double tertiaryBasePrice;
// Perform long computation.
}
}
Solution
Transform the method into a separate class so that the local variables become fields of the class. Then you can split the method into several methods within the same class.
class Order {
// ...
public double price() {
return new PriceCalculator(this).compute();
}
}
class PriceCalculator {
private double primaryBasePrice;
private double secondaryBasePrice;
private double tertiaryBasePrice;
public PriceCalculator(Order order) {
// Copy relevant information from the
// order object.
}
public double compute() {
// Perform long computation.
}
}
Decompose Conditional
Conditional operators and loops are a good clue that code can be moved to a separate method. For conditionals, use Decompose Conditional. If loops are in the way, try Extract Method.
Problem
You have a complex conditional (
if-then
/else
orswitch
).
if (date.before(SUMMER_START) || date.after(SUMMER_END)) {
charge = quantity * winterRate + winterServiceCharge;
}
else {
charge = quantity * summerRate;
}
Solution
Decompose the complicated parts of the conditional into separate methods: the condition,
then
andelse
.
if (isSummer(date)) {
charge = summerCharge(quantity);
}
else {
charge = winterCharge(quantity);
}
Extract Method
Conditional operators and loops are a good clue that code can be moved to a separate method. For conditionals, use Decompose Conditional. If loops are in the way, try Extract Method.
Problem
You have a code fragment that can be grouped together.
void printProperties(List users) {
for (int i = 0; i < users.size(); i++) {
String result = "";
result += users.get(i).getName();
result += " ";
result += users.get(i).getAge();
System.out.println(result);
// ...
}
}
Solution
Move this code to a separate new method (or function) and replace the old code with a call to the method.
void printProperties(List users) {
for (User user : users) {
System.out.println(getProperties(user));
// ...
}
}
String getProperties(User user) {
return user.getName() + " " + user.getAge();
}
Payoff
Among all types of object-oriented code, classes with short methods live longest. The longer a method or function is, the harder it becomes to understand and maintain it.
In addition, long methods offer the perfect hiding place for unwanted duplicate code.
Large Class
A class contains many fields/methods/lines of code.
Classes usually start small. But over time, they get bloated as the program grows. As is the case with long methods as well, programmers usually find it mentally less taxing to place a new feature in an existing class than to create a new class for the feature.
Treatment
When a class is wearing too many (functional) hats, think about splitting it up.
Extract Class
Extract Class helps if part of the behavior of the large class can be spun off into a separate component.
Problem
When one class does the work of two, awkwardness results.
Solution
Instead, create a new class and place the fields and methods responsible for the relevant functionality in it.
Extract Subclass
Extract Subclass helps if part of the behavior of the large class can be implemented in different ways or is used in rare cases.
Problem
A class has features that are used only in certain cases.
Solution
Create a subclass and use it in these cases.
Extract Interface
Extract Interface helps if it’s necessary to have a list of the operations and behaviors that the client can use.
Problem
Multiple clients are using the same part of a class interface. Another case: part of the interface in two classes is the same.
Solution
Move this identical portion to its own interface.
Separate GUI and Domain Data
If a large class is responsible for the graphical interface, you may try to move some of its data and behavior to a separate domain object. In doing so, it may be necessary to store copies of some data in two places and keep the data consistent. Duplicate Observed Data offers a way to do this.
Problem
Is domain data stored in classes responsible for the GUI?
Solution
Then it’s a good idea to separate the data into separate classes, ensuring connection and synchronization between the domain class and the GUI.
Payoff
- Refactoring of these classes spares developers from needing to remember a large number of attributes for a class.
- In many cases, splitting large classes into parts avoids duplication of code and functionality.
Primitive Obsession
Like most other smells, primitive obsessions are born in moments of weakness. “Just a field for storing some data!” the programmer said. Creating a primitive field is so much easier than making a whole new class, right? And so it was done. Then another field was needed and added in the same way. Lo and behold, the class became huge and unwieldy.
Primitives are often used to “simulate” types. So instead of a separate data type, you have a set of numbers or strings that form the list of allowable values for some entity. Easy-to-understand names are then given to these specific numbers and strings via constants, which is why they’re spread wide and far.
Another example of poor primitive use is field simulation. The class contains a large array of diverse data and string constants (which are specified in the class) are used as array indices for getting this data.
Treatment
Replace Set of Fields with Object
If you have a large variety of primitive fields, it may be possible to logically group some of them into their own class. Even better, move the behavior associated with this data into the class too. For this task, try Replace Data Value with Object.
Problem
A class (or group of classes) contains a data field. The field has its own behavior and associated data.
Solution
Create a new class, place the old field and its behavior in the class, and store the object of the class in the original class.
Replace Type Code with Class
When complicated data is coded in variables, use Replace Type Code with Class, Replace Type Code with Subclasses or Replace Type Code with State/Strategy.
Problem
A class has a field that contains type code. The values of this type aren’t used in operator conditions and don’t affect the behavior of the program.
Solution
Create a new class and use its objects instead of the type code values.
Replace Type Code with Subclasses
When complicated data is coded in variables, use Replace Type Code with Class, Replace Type Code with Subclasses or Replace Type Code with State/Strategy.
Problem
You have a coded type that directly affects program behavior (values of this field trigger various code in conditionals).
Solution
Create subclasses for each value of the coded type. Then extract the relevant behaviors from the original class to these subclasses. Replace the control flow code with polymorphism.
Replace Type Code with State/Strategy
When complicated data is coded in variables, use Replace Type Code with Class, Replace Type Code with Subclasses or Replace Type Code with State/Strategy.
Problem
You have a coded type that affects behavior but you can’t use subclasses to get rid of it.
Solution
Replace type code with a state object. If it’s necessary to replace a field value with type code, another state object is “plugged in”.
Replace Array with Object
If there are arrays among the variables, use Replace Array with Object.
Problem
You have an array that contains various types of data.
String[] row = new String[2];
row[0] = "Liverpool";
row[1] = "15";
Solution
Replace the array with an object that will have separate fields for each element.
Performance row = new Performance();
row.setName("Liverpool");
row.setWins("15");
Payoff
- Code becomes more flexible thanks to use of objects instead of primitives.
- Better understandability and organization of code. Operations on particular data are in the same place, instead of being scattered. No more guessing about the reason for all these strange constants and why they’re in an array.
- Easier finding of duplicate code.
Long Parameter List
A long list of parameters might happen after several types of algorithms are merged in a single method. A long list may have been created to control which algorithm will be run and how.
Long parameter lists may also be the byproduct of efforts to make classes more independent of each other. For example, the code for creating specific objects needed in a method was moved from the method to the code for calling the method, but the created objects are passed to the method as parameters. Thus the original class no longer knows about the relationships between objects, and dependency has decreased. But if several of these objects are created, each of them will require its own parameter, which means a longer parameter list.
It’s hard to understand such lists, which become contradictory and hard to use as they grow longer. Instead of a long list of parameters, a method can use the data of its own object. If the current object doesn’t contain all necessary data, another object (which will get the necessary data) can be passed as a method parameter.
Treatment
Replace Parameter with Method Call
Check what values are passed to parameters. If some of the arguments are just results of method calls of another object, use Replace Parameter with Method Call. This object can be placed in the field of its own class or passed as a method parameter.
Problem
Calling a query method and passing its results as the parameters of another method, while that method could call the query directly.
int basePrice = quantity * itemPrice;
double seasonDiscount = this.getSeasonalDiscount();
double fees = this.getFees();
double finalPrice = discountedPrice(basePrice, seasonDiscount, fees);
Solution
Instead of passing the value through a parameter, try placing a query call inside the method body.
int basePrice = quantity * itemPrice;
double finalPrice = discountedPrice(basePrice);
Payoff
- More readable, shorter code.
- Refactoring may reveal previously unnoticed duplicate code.
Data Clumps
Sometimes different parts of the code contain identical groups of variables (such as parameters for connecting to a database). These clumps should be turned into their own classes.
Often these data groups are due to poor program structure or “copypasta programming”.
If you want to make sure whether or not some data is a data clump, just delete one of the data values and see whether the other values still make sense. If this isn’t the case, this is a good sign that this group of variables should be combined into an object.
Treatment
Extract Class
If repeating data comprises the fields of a class, use Extract Class to move the fields to their own class.
Problem
When one class does the work of two, awkwardness results.
Solution
Instead, create a new class and place the fields and methods responsible for the relevant functionality in it.
Payoff
- Improves understanding and organization of code. Operations on particular data are now gathered in a single place, instead of haphazardly throughout the code.
- Reduces code size.
Object-Orientation Abusers
All these smells are incomplete or incorrect application of object-oriented programming principles.
Switch Statements
You have a complex switch
operator or sequence of if
statements.
Relatively rare use of switch
and case
operators is one of the hallmarks of object-oriented code. Often code for a single switch
can be scattered in different places in the program. When a new condition is added, you have to find all the switch
code and modify it.
As a rule of thumb, when you see switch
you should think of polymorphism.
Treatment
Extract Method
To isolate `switch` and put it in the right class, you may need Extract Method and then Move Method.
Problem
You have a code fragment that can be grouped together.
class Order {
// ...
public double calculateTotal() {
double total = 0;
for (Product product : getProducts()) {
total += product.quantity * product.price;
}
// Apply regional discounts.
switch (user.getCountry()) {
case "US": total *= 0.85; break;
case "RU": total *= 0.75; break;
case "CN": total *= 0.9; break;
// ...
}
return total;
}
Solution
Move this code to a separate new method (or function) and replace the old code with a call to the method.
class Order {
// ...
public double calculateTotal() {
double total = 0;
for (Product product : getProducts()) {
total += product.quantity * product.price;
}
total = applyRegionalDiscounts(total);
return total;
}
public double applyRegionalDiscounts(double total) {
double result = total;
switch (user.getCountry()) {
case "US": result *= 0.85; break;
case "RU": result *= 0.75; break;
case "CN": result *= 0.9; break;
// ...
}
return result;
}
Move Method
To isolate `switch` and put it in the right class, you may need Extract Method and then Move Method.
Problem
A method is used more in another class than in its own class.
class Order {
// ...
public double calculateTotal() {
double total = 0;
for (Product product : getProducts()) {
total += product.quantity * product.price;
}
total = applyRegionalDiscounts(total);
return total;
}
public double applyRegionalDiscounts(double total) {
double result = total;
switch (user.getCountry()) {
case "US": result *= 0.85; break;
case "RU": result *= 0.75; break;
case "CN": result *= 0.9; break;
// ...
}
return result;
}
Solution
Create a new method in the class that uses the method the most, then move code from the old method to there. Turn the code of the original method into a reference to the new method in the other class or else remove it entirely.
class Order {
// ...
public double calculateTotal() {
// ...
total = Discounts.applyRegionalDiscounts(total, user.getCountry());
total = Discounts.applyCoupons(total);
// ...
}
class Discounts {
// ...
public static double applyRegionalDiscounts(double total, string country) {
double result = total;
switch (country) {
case "US": result *= 0.85; break;
case "RU": result *= 0.75; break;
case "CN": result *= 0.9; break;
// ...
}
return result;
}
public static double applyCoupons(double total) {
// ...
}
Replace Conditional with Polymorphism
After specifying the inheritance structure, use Replace Conditional with Polymorphism.
Problem
You have a conditional that performs various actions depending on object type or properties.
class Bird {
// ...
double getSpeed() {
switch (type) {
case EUROPEAN:
return getBaseSpeed();
case AFRICAN:
return getBaseSpeed() - getLoadFactor() * numberOfCoconuts;
case NORWEGIAN_BLUE:
return (isNailed) ? 0 : getBaseSpeed(voltage);
}
throw new RuntimeException("Should be unreachable");
}
}
Solution
Create subclasses matching the branches of the conditional. In them, create a shared method and move code from the corresponding branch of the conditional to it. Then replace the conditional with the relevant method call. The result is that the proper implementation will be attained via polymorphism depending on the object class.
abstract class Bird {
// ...
abstract double getSpeed();
}
class European extends Bird {
double getSpeed() {
return getBaseSpeed();
}
}
class African extends Bird {
double getSpeed() {
return getBaseSpeed() - getLoadFactor() * numberOfCoconuts;
}
}
class NorwegianBlue extends Bird {
double getSpeed() {
return (isNailed) ? 0 : getBaseSpeed(voltage);
}
}
// Somewhere in client code
speed = bird.getSpeed();
Replace Parameter with Explicit Methods
If there aren’t too many conditions in the operator and they all call same method with different parameters, polymorphism will be superfluous. If this case, you can break that method into multiple smaller methods with Replace Parameter with Explicit Methods and change the `switch` accordingly.
Problem
A method is split into parts, each of which is run depending on the value of a parameter.
void setValue(String name, int value) {
if (name.equals("height")) {
height = value;
return;
}
if (name.equals("width")) {
width = value;
return;
}
Assert.shouldNeverReachHere();
}
Solution
Extract the individual parts of the method into their own methods and call them instead of the original method.
void setHeight(int arg) {
height = arg;
}
void setWidth(int arg) {
width = arg;
}
Introduce Null Object
If one of the conditional options is null
, use Introduce Null Object.
Problem
Since some methods return
null
instead of real objects, you have many checks fornull
in your code.
if (customer == null) {
plan = BillingPlan.basic();
}
else {
plan = customer.getPlan();
}
Solution
Instead of
null
, return a null object that exhibits the default behavior.
class NullCustomer extends Customer {
boolean isNull() {
return true;
}
Plan getPlan() {
return new NullPlan();
}
// Some other NULL functionality.
}
// Replace null values with Null-object.
customer = (order.customer != null) ?
order.customer : new NullCustomer();
// Use Null-object as if it's normal subclass.
plan = customer.getPlan();
Payoff
- Improved code organization.
When to Ignore
- When a
switch
operator performs simple actions, there's no reason to make code changes. - Often
switch
operators are used by factory design patterns (Factory Method or Abstract Factory) to select a created class.
Temporary Field
Temporary fields get their values (and thus are needed by objects) only under certain circumstances. Outside of these circumstances, they’re empty.
Oftentimes, temporary fields are created for use in an algorithm that requires a large amount of inputs. So instead of creating a large number of parameters in the method, the programmer decides to create fields for this data in the class. These fields are used only in the algorithm and go unused the rest of the time.
This kind of code is tough to understand. You expect to see data in object fields but for some reason they’re almost always empty.
Refused Bequest
If a subclass uses only some of the methods and properties inherited from its parents, the hierarchy is off-kilter. The unneeded methods may simply go unused or be redefined and give off exceptions.
Someone was motivated to create inheritance between classes only by the desire to reuse the code in a superclass. But the superclass and subclass are completely different.
Treatment
Replace Inheritance with Delegation
If inheritance makes no sense and the subclass really does have nothing in common with the superclass, eliminate inheritance in favor of Replace Inheritance with Delegation.
Problem
You have a subclass that uses only a portion of the methods of its superclass (or it’s not possible to inherit superclass data).
Solution
Create a field and put a superclass object in it, delegate methods to the superclass object, and get rid of inheritance.
Extract Superclass
If inheritance is appropriate, get rid of unneeded fields and methods in the subclass. Extract all fields and methods needed by the subclass from the parent class, put them in a new superclass, and set both classes to inherit from it (Extract Superclass).
Problem
You have two classes with common fields and methods.
Solution
Create a shared superclass for them and move all the identical fields and methods to it.
Payoff
- Improves code clarity and organization. You will no longer have to wonder why the
Dog
class is inherited from theChair
class (even though they both have 4 legs).
Alternative Classes with Different Interfaces
Two classes perform identical functions but have different method names.
The programmer who created one of the classes probably didn’t know that a functionally equivalent class already existed.
Treatment
Rename Method
Rename Methods to make them identical in all alternative classes.
Problem
The name of a method doesn’t explain what the method does.
Solution
Rename the method.
Move Method
Move Method, Add Parameter and Parameterize Method to make the signature and implementation of methods the same.
Problem
A method is used more in another class than in its own class.
Solution
Create a new method in the class that uses the method the most, then move code from the old method to there. Turn the code of the original method into a reference to the new method in the other class or else remove it entirely.
Add Parameter
Move Method, Add Parameter and Parameterize Method to make the signature and implementation of methods the same.
Problem
A method doesn’t have enough data to perform certain actions.
Solution
Create a new parameter to pass the necessary data.
Parameterize Method
Move Method, Add Parameter and Parameterize Method to make the signature and implementation of methods the same.
Problem
Multiple methods perform similar actions that are different only in their internal values, numbers or operations.
Solution
Combine these methods by using a parameter that will pass the necessary special value.
Payoff
- You get rid of unnecessary duplicated code, making the resulting code less bulky.
- Code becomes more readable and understandable (you no longer have to guess the reason for creation of a second class performing the exact same functions as the first one).
When to Ignore
- Sometimes merging classes is impossible or so difficult as to be pointless. One example is when the alternative classes are in different libraries that each have their own version of the class.