Say you come across a class named PollutantEntry that has a long parameter list in the constructor:
class PollutantEntry {
String country;
String state;
String city;
String place;
LocalDateTime localDateTime;
Float average;
Float max;
Float min;
String pollutant;
public PollutantEntry(String country, String state, String city, String place, LocalDateTime localDateTime, Float average, Float max, Float min, String pollutant) {
this.country = country;
this.state = state;
this.city = city;
this.place = place;
this.localDateTime = localDateTime;
this.average = average;
this.max = max;
this.min = min;
this.pollutant = pollutant;
}
}
There are many problems with such parameter lists:
a) The caller code that invokes the constructor is often unreadable. This one is certainly not pretty code:
new PollutantEntry(readingEntries[0], readingEntries[1],
readingEntries[2], readingEntries[3], readingTime, processFloat(readingEntries[5]), processFloat(readingEntries[6]), processFloat(readingEntries[7]), readingEntries[8]);
(Check the overloaded constructors in java.util.Date for example here.)
b) Many of the fields are often optional. Trying to take into account optional parameters through overloaded constructor calls make it only worser with even 8–10 overloaded constructors like this:
public PollutantEntry(String state, String city, String place, LocalDateTime localDateTime, Float average, Float max, Float min, String pollutant) {
this("India", state, city, place, localDateTime, average, max, min, pollutant);
}
Again, not at all nice.
c) When there is validation code involved, the constructor method body can become quite big & complex with high cyclomatic complexity.
You’ll often see clumps of parameters that are always passed & used together (“data clumps”) — that’s another smell.
Bottom-line: For sure, such code smells bad and needs fixing.
There are a couple of approaches for dealing with “long parameter list in constructors” smell.
Option 1: Use builder pattern
Instead of passing the parameters as arguments, construct an object by calling the methods by introducing the Builder pattern. In fact, this refactoring is so easy and common that the IDEs even automate it. Here is the example from IntelliJ IDEA:
Select the constructor with long parameter list -> “Refactor” -> “Replace Constructor with Builder…”
And choose the “Refactor” in the box that pops up:
That’s it — two clicks and you have a builder class created for you:
import java.time.LocalDateTime;
public class PollutantEntryBuilder {
private String country;
private String state;
private String city;
private String place;
private LocalDateTime localDateTime;
private Float average;
private Float max;
private Float min;
private String pollutant;
public PollutantEntryBuilder setCountry(String country) {
this.country = country;
return this;
}
public PollutantEntryBuilder setState(String state) {
this.state = state;
return this;
}
public PollutantEntryBuilder setCity(String city) {
this.city = city;
return this;
}
public PollutantEntryBuilder setPlace(String place) {
this.place = place;
return this;
}
public PollutantEntryBuilder setLocalDateTime(LocalDateTime localDateTime) {
this.localDateTime = localDateTime;
return this;
}
public PollutantEntryBuilder setAverage(Float average) {
this.average = average;
return this;
}
public PollutantEntryBuilder setMax(Float max) {
this.max = max;
return this;
}
public PollutantEntryBuilder setMin(Float min) {
this.min = min;
return this;
}
public PollutantEntryBuilder setPollutant(String pollutant) {
this.pollutant = pollutant;
return this;
}
public PollutantEntry createPollutantEntry() {
return new PollutantEntry(country, state, city, place, localDateTime, average, max, min, pollutant);
}
}
What’s more, the caller code gets automatically replaced with the call to the builder!
PollutantEntry pollutantEntry =
new PollutantEntryBuilder()
.setCountry(readingEntries[0])
.setState(readingEntries[1])
.setCity(readingEntries[2])
.setPlace(readingEntries[3])
.setLocalDateTime(readingTime)
.setAverage(processFloat(readingEntries[5]))
.setMax(processFloat(readingEntries[6]))
.setMin(processFloat(readingEntries[7]))
.setPollutant(readingEntries[8])
.createPollutantEntry();
Not bad, but if you ask me, this still sucks! It’s too long to read now — it looks more like a long Cobra slithering in the code and snakes smell bad as well!
Option 2: Refactor data clumps with abstractions
Let’s take a look at the original PollutantEntry now. There are three logical entities there — location, time, and pollutant name cum readings. Of these, location & pollutant details could be made as separate abstractions. Let’s do that:
public class Location {
private String country;
private String state;
private String city;
private String place;
public Location(String country, String state, String city, String place) {
this.country = country;
this.state = state;
this.city = city;
this.place = place;
}
}public class PollutionData {
private String avg;
private String max;
private String min;
private String pollutant;
public PollutionData(String avg, String max, String min, String pollutant) {
this.avg = avg;
this.min = min;
this.max = max;
this.pollutant = pollutant;
}
}public class PollutantEntry {
private Location location;
private LocalDateTime lastUpdate;
private PollutionData pollutionData;
public PollutantEntry(Location location, LocalDateTime lastUpdate, PollutionData pollutionData) {
this.location = location;
this.lastUpdate = lastUpdate;
this.pollutionData = pollutionData;
}
}
Hmm, this does get rid of the long parameter list in the constructor and looks elegant. But can we do better?!
Option 3: Creating Abstractions and Use Builder
In this approach, let’s create the abstractions and use the Builder pattern, as in:
public class PollutantEntry {
private Location location;
private LocalDateTime lastUpdate;
private PollutionData pollutionData;
/* The Builder class corresponds to the PollutantEntry - it helps create a Pollutant entry object given the location object, time of reading, and the actual pollution reading */
public static class Builder {
PollutantEntry pollutantEntry = new PollutantEntry();
public Builder() {
}
public Builder location(Location location) {
pollutantEntry.location = location;
return this;
}
public Builder lastUpdate(LocalDateTime lastUpdate) {
pollutantEntry.lastUpdate = lastUpdate;
return this;
}
public Builder pollutionData(PollutionData pollutionData) {
pollutantEntry.pollutionData = pollutionData;
return this;
}
public PollutantEntry build() {
return pollutantEntry;
}
}
In this case, we don’t have a constructor (or make the constructor private) and choose a static nested Builder class. Now the caller code looks like this:
PollutantEntry pollutantEntry =
new PollutantEntry.Builder()
.location(location)
.lastUpdate(time)
.pollutionData(pollutionData)
.build();
Ah, I love this — short, sweet and elegant — as any code should be :-)
An example of use of Builder pattern in Java class library is here:
Calendar calendar = new Calendar.Builder()
.setDate(2019, 9, 10)
.build();
Nice, isn’t it? Imagine providing a constructor for Calendar with so many options like day, month, year, timezone, and what not. Instead, this builder pattern nicely exposes the relevant fields as methods that we can set as needed.
Summary
Long parameter list in constructor is a well-known design smell. To address this smell, don’t look for overloaded constructors as a solution — I would say that’s another smell! Rather, look out for data clumps in the parameters are create abstractions. Also check if introducing Builder pattern can help. Better still, see if you can use both options — introducing abstractions and Builder pattern — that helps refactor this smell into cleaner code.
(Written by Ganesh Samarthyam, Co-Founder, KonfHub Technologies LLP)