Misplaced Responsibility: Mutant Builders

The Builder Pattern provides us with a useful way to instantiate objects. Builders decouple our code from concrete implementations. Builders make our code more readable, especially when used with a fluent interface. Builders make our code more cohesive, particularly when creating complex objects. The Builder Pattern is a very powerful part of a developers took kit.

So when I found a Builder that was crafted in such a way that the object it "built" could unexpectedly mutate, I found myself referring to it as a Mutant Builder.

mutant-builder.jpeg

Let's look at an example to get an idea of what I'm talking about.

PersonBuilder builder = new PersonBuilder();

builder.with(name("Thomas"));
Person thomas = builder.build();

builder.with(name("Mary"));
Person mary = builder.build();

So far so good. We should have three objects, a PersonBuilder, and two Person objects, thomas, and mary. But then this happens:

// This will output "Thomas's name is: Mary"
System.out.println("Thomas's name is: " + thomas.getName());
// This will output "Mary's name is: Mary"
System.out.println("Mary's name is: " + mary.getName());

This Builder is not behaving itself. It is causing side-effects in an unrelated object, it is a Mutant Builder, and it is mutating Thomas (poor chap). You've probably guessed the reason, we don't really have three objects, we have two; thomas, and mary are one and the same.

Let's take a look at PersonBuilder:

public class PersonBuilder {

    private Person person = new Person();

    public PersonBuilder with(Name name) {
        person.setName(name);
        return this;
    }

    public Person build() {
        return person;
    }

}

The cause is simple, Misplaced Responsibility, a code smell. The build() method does not do what it says on the tin, it does not build a new Person. This Builder can only create one Person object. Net result, when we reuse an existing PersonBuilder, we will inadvertently alter Person objects we think we have previously built. If the PersonBuilder is used once and thrown away, the problem is never exposed. For example:

Person thomas = new PersonBuilder().with(name("Thomas")).build();
Person mary = new PersonBuilder().with(name("Mary")).build();

This time, with two separate PersonBuilder objects, we work around the problem. Of course this is not a workable solution. So let's do the right thing, let's fix the PersonBuilder.

public class PersonBuilder {

    private Name name;

    public PersonBuilder with(Name name) {
        this.name = name;
        return this;
    }

    public Person build() {
        return new Person(name);
    }

}

The Misplaced Responsibility code smell has been eradicated. The responsibility of instantiating Person objects has been relocated to the build() method. Better still, we no longer experience the Mutant Builder, and Thomas is safe.

Tweet about this on TwitterShare on FacebookShare on LinkedInShare on Google+Share on RedditEmail this to someone
Posted in Bad Code