loading...

When Builder is anti-pattern

siy profile image Sergiy Yevtushenko Updated on ・2 min read

Some time ago I worked on the project which was full of "best practices", applied thoroughly everywhere. One of such "best practices" was mandatory use of Builder to create POJO's. Most of the POJO's boilerplate code was generated by IDE plugin, but it was only tiny portion of code. Actual problem was the use of these POJO's. Code was flooded with pieces like this:

    final MyPojo pojo = MyPojo.newBuilder()
                              .setThis(...)
                              .setThat(...)
                              .setSomethingElse(...)
                              ...
                              .build();

Such an overuse of the builders has several negative effects:

  • code is unnecessarily verbose (I'd even say heavily noisy)
  • code is error prone

While first issue is more matter of taste, second one is not:

There is no way to tell at compile time if all necessary fields are set.

Plain all-args constructor or factory method works better in this case - compiler will make sure that I'll pass all parameters necessary to create POJO. How correct are these parameters it's the another story, but all of them definitely will be there. With builder I can easily omit one or two. Issue will appear later, at run time, when I get NPE.

The bright mind who introduced that practice in mentioned above project, decided that best way to solve the problem is to add validation into the build() method. It resulted that instead of getting NPE's in random locations, we started getting them inside build() methods. Nice improvement, isn't it?

The described above anti-pattern is an example of wider class of heavy design mistakes - shifting errors from compile time to run time. Such a shift is call for troubles of any size, from longer and more painful development to huge losses for those who uses the application.

So, what's the correct use of Builder?

My criteria of correct use of Builder pattern is extremely simple:
The build() method must produce complete instance. All necessary fields must be set to reasonable defaults if user did not provide values for them.

POJO's rarely fall into category of classes which can be built this way. Usually POJO has most or all fields necessary to represent information which just does not have reasonable default values.

Another place where Builder is often used are things like server or client configurations and usually Builder's are perfectly fine to this kind of applications.

Posted on by:

siy profile

Sergiy Yevtushenko

@siy

Writing code for 30+ years and still enjoy it...

Discussion

markdown guide
 

Let's say my POJO has 20 fields, would I create a factory method with so many arguments? Would it be less error prone than a builder?

 

I think in this case it worth to figure out why this POJO has so many fields first. Such POJO's are code smell by itself.

 

A POJO representing a database table record, for example.

Such a DB design is also smell. But sometimes it's hard to change. There is no universal recipe what to do, but you can try following things:

  • group linked fields into more complex objects (like extract all user details into user object + contact info object)
  • use projections - narrower POJO's for specific use cases
  • kinda inverse version of projections - create core POJO which contains most often used fields and several specific POJO's for various use cases (and use core POJO as single field) The goal of all these approaches is the same - reduce number of fields. This simplifies use of such a big POJO and in the same time reduces risk of error.

You mean a database table with 20 columns is a bad smell too? I don't like them but they're not uncommon. And forget about 20, 10 columns (and a 10 parameter method) is too much too.

Reading builders can be easier than reading factory methods too, they,re the closest thing to named parameters in java.
You can argue they involve too many lines of code but the readability is better than with factory methods (IMO).

20 DB columns not always correspond directly to 20 fields in POJO.

And yes, I understand that this is about readability. I just think that price is too high. There is a solution which does not as error-prone as Builder, but it involves even more boilerplate code than Builder and there is no ready to use code generator.

No option's perfect, I guess.
As a matter of fact, I don't always use Builders or Factories, same old getters/setters are enough most of the time (immutable objects are not always needed).

Well, there is no silver bullet anyway.

 
 

This is interesting, what are the alternatives?

 

There is a pattern which allows to solve all these issues. Usually it is involved for creating DSL's to limit possible choices and avoid user mistakes. The idea of the pattern is quite simple: when builder is created, it returns interface which has limited number of methods to call and does not have final build() method. Methods of first interface return other interfaces with other set of methods and so on an so forth, until last interface is reached, which has build() method.

This pattern allows to avoid Builder pattern issues (and even allows to enforce code style, for example make all invocations to use same order of setting field values), but it's implementation is somewhat verbose. For DSL's this is perfectly OK, but for building POJO's it might be overkill.

I'm going to describe this pattern in one of the next posts.

 

So it's more like a Buildup pattern? 😊

Basically yes. Thanks for naming suggestion :)

 

I believe what you are describing is commonly referred to as a fluent interface.

Fluent interface is orthogonal to the pattern described above.

I don't see how? A fluent interface attempts to guide you through a process without you really having to read the documentation (you feel like you are already fluent in the API). For example, I could have a class like ClientFactory with a single method createClient(). The return from that method could be an object with two methods forAmazon() or forGoogle(). Depending on which method you call you may have different options for how to have the client authenticate. The fluent API gives you limited valid options until you hit a terminal method that finally builds and returns the object you are looking for. It sounds exactly like what you were describing in your DSL like solution above.

Out of curiosity I've checked what Wikipedia says about fluent interface:

Note that a "fluent interface" means more than just method cascading via chaining; it entails designing an interface that reads like a DSL, using other techniques like "nested functions and object scoping".

So, seems I was wrong about orthogonality. From the other hand, looks like fluent interface is designed to be "DSL-like", so basically it's just two different names for same thing.

 

Alternatives to builders are:

  • Factories or Factory methods
  • The Interpreter pattern

The interpreter pattern is used in things such as building HTML or something like that. Then you can run a visitor through it to generate some end result.

Builders have their place, like Wizards, but as anything they shouldn't be overused.

 

I think it depends on use case. Sometimes it might be static factory method, sometimes you might have no need to build full object manually at all (in case of DB mapping, for example).

 

Builders are horrible. I've used them mainly in SQL query builder context, but also seen the things you described in the wild.

It's definitely a clear indication of design problems when some builders start popping in the code base. If you're objects are so hard to create that you need some builder class to create those, you need to take a look at the classes you're creating and fix the problem there.

Factories using reasonable parameter objects are easier to work with and also easier to test even when the mistake of too big classes has already been made.

 

Yeah, back in the day when we'd manually build our own SQL a builder pattern was very useful. I suppose other use cases could be creating other text formats. Or the obvious application of a builder is for Wizards. A wizard is a set of steps and the builder is the most natural way to feed those steps until you're ready to run.

 

Good points, especially about shifting away from compile time to run time.
I like to use the builder pattern as an abstraction layer in cases where I need to create/build a complex request, like a deeply nested highly verbose xml message. When you need to communicate a simple value, but the datacontract is highly verbose in how you need to communicate it, like the same value in 10 different elements. Or when the message is highly generalized and devoid of the business domain.

 

In my opinion, the title you gave is interesting. In your example, you describe how was the basic rule of this pattern violated and what was the outcome. The basic rule of the builder pattern is that it always allows producing the ”right” object. It means that event without passing the required parameter to the constructor, the object should be produced with some default value.

I use the builder pattern, especially in my unit testes, where I instantiate a lot of objects in a test suit. When I add a new parameter to the object’s constructor, I have only one place to change in the code (instead of changing all lines with the construction method).

To sum up, I agree that the builder is an anti-pattern when you don’t know how to use it. I think it is a general rule for all patterns.

 

You might notice in the article that I exactly know how to use the pattern.
The article focuses on the pattern use case which I often see in projects. The same case is default behaviour of various IDE plugins. So yes, the problem is with particular (incorrect) use of the Builder and my article (and title) is an attempt to draw attention to that incorrect use of pattern.

In general the blind following of "best practices" is a significant problem in modern Java (and not just Java) development. I'm trying to draw attention to this problem and provide arguments to make use of "best practices" meaningful and justified.

 

"There is no way to tell at compile time if all necessary fields are set." - I completely agree. I've also noticed this a long time ago. This is exactly why builder is my "favourite" (anti)pattern.