Java Design Patterns – Why Types Are Coupled with Their Builders

builder-patterncouplingdesign-patternsjava

I've recently deleted a answer of mine on Code Review, that started like this:

private Person(PersonBuilder builder) {

Stop. Red flag. A PersonBuilder would build a Person; it knows about a Person. The Person class shouldn't know anything about a PersonBuilder – it's just an immutable type. You've created a circular coupling here, where A depends on B which depends on A.

The Person should just intake its parameters; a client that's willing to create a Person without building it should be able to do that.

I was slapped with a downvote, and told that (quoting) Red flag, why? The implementation here has the same shape that which Joshua Bloch's demonstrated in his "Effective Java" book(item #2).

So, it appears that the One Right Way of implementing a Builder Pattern in Java is to make the builder a nested type (this isn't what this question is about though), and then make the product (the class of the object being built) take a dependency on the builder, like this:

private StreetMap(Builder builder) {
    // Required parameters
    origin      = builder.origin;
    destination = builder.destination;

    // Optional parameters
    waterColor         = builder.waterColor;
    landColor          = builder.landColor;
    highTrafficColor   = builder.highTrafficColor;
    mediumTrafficColor = builder.mediumTrafficColor;
    lowTrafficColor    = builder.lowTrafficColor;
}

https://en.wikipedia.org/wiki/Builder_pattern#Java_example

The same Wikipedia page for the same Builder pattern has a wildly different (and much more flexible) implementation for C#:

//Represents a product created by the builder
public class Car
{
    public Car()
    {
    }

    public int Wheels { get; set; }

    public string Colour { get; set; }
}

As you can see, the product here does not know anything about a Builder class, and for all it cares it could be instantiated by a direct constructor call, an abstract factory, …or a builder – as far as I understand it, the product of a creational pattern never needs to know anything about what's creating it.

I've been served the counter-argument (which is apparently explicitly defended in Bloch's book) that a builder pattern could be used to rework a type that would have a constructor bloated with dozens of optional arguments. So instead of sticking to what I thought I knew I researched a bit on this site, and found that as I suspected, this argument does not hold water.

So what's the deal? Why come up with over-engineered solutions to a problem that shouldn't even exist in the first place? If we take Joshua Bloch off his pedestal for a minute, can we come up with one single good, valid reason for coupling two concrete types and calling it a best practice?

This all reeks of cargo-cult programming to me.

Best Answer

I disagree with your assertion that it is a red flag. I also disagree with your representation of the counterpoint, that there is One Right Way of representing a Builder pattern.

To me there's one question: Is the Builder a necessary part of the type's API? Here, is the PersonBuilder a necessary part of the Person API?

It is entirely reasonable that a validity-checked, immutable, and/or tightly-encapsulated Person class will necessarily be created through a Builder it provides (regardless of whether that builder is nested or adjacent). In doing so, the Person can keep all of its fields private or package-private and final, and also leave the class open for modification if it is a work in progress. (It may end up closed for modification and open for extension, but that's not important right now, and is especially controversial for immutable data objects.)

If it is the case that a Person is necessarily created through a supplied PersonBuilder as part of a single package-level API design, then a circular coupling is just fine and a red flag isn't warranted here. Notably, you stated it as an incontrovertible fact and not an API design opinion or choice, so that may have contributed to a bad response. I wouldn't have downvoted you, but I don't blame someone else for doing so, and I'll leave the rest of the discussion of "what warrants a downvote" to the Code Review help center and meta. Downvotes happen; it's no "slap".

Of course, if you want to leave a lot of ways open to build a Person object, then the PersonBuilder could become a consumer or utility of the Person class, on which the Person should not directly depend. This choice seems more flexible—who wouldn't want more object creation options?—but in order to hold to guarantees (like immutability or serializability) suddenly the Person has to expose a different kind of public creation technique, such as a very long constructor. If the Builder is meant to represent or replace a constructor with a lot of optional fields or a constructor open to modification, then the class's long constructor may be an implementation detail better left hidden. (Also bear in mind that an official and necessary Builder doesn't preclude you from writing your own construction utility, just that your construction utility may consume the Builder as an API as in my original question above.)

p.s.: I note that the code review sample you listed has an immutable Person, but the counterexample from Wikipedia lists a mutable Car with getters and setters. It may be easier to see the necessary machinery omitted from the Car if you keep the language and invariants consistent between them.

Related Topic