Lately I stumble a lot upon code I wrote 4 or more years ago. In the light of introducing new features the code gets tested for its quality. One antipattern I’ve found which I had used in the past but which is really hard to extend is convenience constructors. Take a constructor for a command object for example:
public SetProperty(String filename, String key, String value) {
this(filename, key, value, null);
}
public SetProperty(String filename,
String key, String value, String comment) {
this(filename, ReferenceTo.key(key), value, comment);
}
public SetProperty(String filename,
String sectionType, String sectionName,
String key, String value) {
this(filename, sectionType, sectionName, key, value, null);
}
public SetProperty(String filename,
String sectionType, String sectionName,
String key, String value, String comment) {
this(filename, ReferenceTo.sectionAndKey(sectionType, sectionName, key), value, comment);
}
public SetProperty(String filename,
AdvancedPropertyReference propertyReference,
String value, String comment) {
this(filename, propertyReference, value, comment);
}
public SetProperty(String filename,
AdvancedPropertyReference propertyReference,
String value, String comment) {
super(filename);
this.propertyReference = propertyReference;
this.value = value;
this.comment = comment;
}
We need to add a new feature which enables us to append properties not just set and replace them. One way could be to extend the class. But this is overkill. Just adding a new parameter flag should suffice. But this would blow up the number of constructors because you need to include a version with and without the new parameter for each (used) constructor. Here an old friend comes to the rescue: design patterns. Looking at the GoF book shows a good solution to the problem: the builder pattern.
public class SetPropertyBuilder {
private final String filename;
private String sectionType;
private String sectionName;
private String referenceKey;
private String value;
private String comment;
private boolean append;
public SetPropertyBuilder(String filename) {
super();
this.filename = filename;
}
public SetPropertyBuilder set(String key, String newValue) {
this.referenceKey = key;
this.value = newValue;
return this;
}
public SetPropertyBuilder append(String key, String additionalValue) {
set(key, additionalValue);
this.append = true;
return this;
}
public SetPropertyBuilder inSection(String type, String name) {
this.sectionType = type;
this.sectionName = name;
return this;
}
public SetProperty build() {
AdvancedPropertyReference reference = ReferenceTo.key(this.referenceKey);
if (this.sectionType != null && this.sectionName != null) {
reference = ReferenceTo.sectionAndKey(this.sectionType, this.sectionName, this.referenceKey);
}
return new SetProperty(this.filename, reference, this.value, this.comment, this.append);
}
}
Now we can eleminate all but one constructor from the SetProperty command. Adding a new property now yields one new method in the builder.
Schneide Blog 
Nice post – although one could argue that the Builder-Pattern is an antipattern in itself. My theory regarding this is twofold:
1. The builder is often used when the process of creating an object is rather complex. So instead of making the creation process simpler by refactoring or hard thinking you just put the complicated stuff in another class and call it a builder. Awesome. Problem solved. Isn’t the use of a builder just hiding the problem?
2. You always have to think hard about what you are doing when trading complexity for something else. If someone wants to use your class a convenience constructor seems to be the a good thing to get this person started. Execute one constructor – boom you have your instance. Introducing a new (builder) class – just to encapsulate the creation of a object – adds complexity for the developer who just wants to use your class. The most pragmatic solution would be to ignore the existence of the builder pattern which is for example done by a lot of Objective-C developers. 🙂
Are you implying the builder pattern is a cure for a symptom not for the problem itself? Yes, that could be. Especially the GoF design patterns are an indicator for missing language features or a clunky language. (see Norvigs paper about GoF patterns in dynamical typed languages or the recent discussion about the GoF patterns in functional ones)
In this example the convenience constructors emerged from having optional parameters. So in a language where optional parameters are supported you have fewer constructors.
Maybe the problem is in your architecture, many service based architectures need some services injected into other services or objects. This could also be a case of over applying the SRP (single responsibility principle) and modularizing too much.
Yes – thats what I am implying. 🙂
Thanks for the reference to the paper.
Yes – thats what I am implying. 🙂
Thanks for the reference to the paper.
Considering the Java builder pattern to be a code smell is a bit harsh in my view. Sometimes you can’t avoid having objects with a lot of interdependent attributes especially in domain classes mirroring “weird” business requirements. Breaking up these objects into smaller ones might be a solution or just a different sort of headache, when it comes to the Law of Demeter.
On advantage of the Builder pattern is rarely mentioned: It allows you to keep the validation within the class itself. If the exceptions carry enough information there shouldn’t be a need for additional validation inside the presentation layer.
I like the version presented here: http://egalluzzo.blogspot.de/2010/06/using-inheritance-with-fluent.html
It basically splits up a POJO class with getters and setters into two classes with setters in one class and getters in the other one. Constructors are completely left out of the equation when adding a new attribute. Looking at the relatively small overhead, i dare say immutability doesn’t get much cheaper than this.
Where did I say the builder pattern is a code smell? I think convenience constructors are a code smell and the builder pattern is a possible solution here.
You didn’t, but Christian even used the term “antipattern” in his first response. My apologies for not replying to his post.
Btw: was the SetProperty class supposed to be immutable or not?
Yes – I think that the builder pattern is a code smell. For me a code smell is not a definitive ruling. If I would have to come up with a definition of what a code smell is then I would say that it indicates a possible problem. For example: If you create a singleton this is a code smell. Not because singletons are the wrong choice all the time but because the are the wrong choice most of the time. There are cases where singletons make sense. I think the same is true for the builder pattern. It is the wrong choice most of the times and thus can be called a code smell because it indicates a possible problem which may exist or not exist. Frisian: Basically I totally agree with what you were saying:
“Sometimes you can’t avoid having objects with a lot of interdependent attributes especially in domain classes mirroring “weird” business requirements. Breaking up these objects into smaller ones might be a solution or just a different sort of headache, when it comes to the Law of Demeter.”
Agreed. But then again: How often do you have these “weird” business requirements? I guess that this depends heavily on the industry you are developing for. What I was trying to say: The builder pattern is overused. Lets take the example given in this blog post. What kind of weird business requirements are needed for someone to create a SetPropertyBuilder class?
@Christian:
The example given contains seven attributes. Compare that to how many attributes you need for a proper international address and we’re not even at the “weird” stuff.
Of the seven attributes five are Strings, which will make reading an argument list cumbersome at best. I favour immutability, so if I don’t want to write a lot of boilerplate code, there will have to be constructors or equivalent static creation methods covering all possible attributes in the absence of a Builder class. This a Java thing. Java doesn’t have optional or named parameters. The construct that comes closest is the Java Builder pattern.
Concerning the “weird” stuff: This isn’t limited to certain industries: Standardized transport contracts have a lot of clauses, which can be included or not. Mechanical specs for a product can be impressive, too. A simple book in an online store can have a lot of attributes (title, author(s), ISBN, # pages, hardcover/paperback, format, publisher, year, edition to name a few). If you’re lucky you can categorize them. If not, you end up with possibly complex business logic to decide, which of the attributes are mandatory and which are optional for a given case.
So, I guess we agree to disagree here: I consider the Java Builder pattern to be a consequence of the Java language itself (no named or optional parameters) and to stay true to business requirements in the code (lots of attributes, which can’t be grouped meaningfully).