Is this a valid Java implementation of an immutable class and the Builder pattern?

Question

The Builder implements Cloneable and overrides clone() and instead of copying every field of the builder, the immutable class keeps a private clone of the builder. This makes it easy to return a new builder and create slightly modified copies of an immutable instance.

This way I can go

MyImmutable i1 = new MyImmutable.Builder().foo(1).bar(2).build();
MyImmutable i2 = i1.builder().foo(3).build();

The Cloneable interface is said to be somewhat broken, but does any of this violate good java coding practice, are there any problems with this construct?

final class MyImmutable { 
  public int foo() { return builder.foo; }
  public int bar() { return builder.bar; }
  public Builder builder() { return builder.clone(); }
  public static final class Builder implements Cloneable {
    public Builder foo(int val) { foo = val; return this; }
    public Builder bar(int val) { bar = val; return this; }
    public MyImmutable build() { return new MyImmutable(this.clone()); }
    private int foo = 0;
    private int bar = 0;
    @Override public Builder clone() { try { return (Builder)super.clone(); } catch(CloneNotSupportedException e) { throw new AssertionError(); } }
  }
  private MyImmutable(Builder builder) { this.builder = builder; }
  private final Builder builder;
}

Answer 1

I've not seen this approach before, but looks like it would work fine.

Basically it makes the builder pattern relatively simple to implement, at the expense of slightly higher runtime overhead (extra objects + cloning operations + a level of indirection in accessor functions that may or may not get compiled out).

A potential variation you might want to consider: if you make the builder objects themselves immutable, you won't need to defensively clone them. This could be an overall win, especially if you build objects a lot more frequently than you change the builders.

Answer 2

Generally the class constructed from the Builder doesn't have any specialized knowledge of the builder. That is Immutable would have a constructor to supply the value for foo and bar:

public final class MyImmutable {
  public final int foo;
  public final int bar;
  public MyImmutable(int foo, int bar) {
    this.foo = foo;
    this.bar = bar;
  }
}

The builder would be a separate class:

public class MyImmutableBuilder {
  private int foo;
  private int bar;
  public MyImmutableBuilder foo(int val) { foo = val; return this; }
  public MyImmutableBuilder bar(int val) { bar = val; return this; }
  public MyImmutable build() { return new MyImmutable(foo, bar); }
}

If you wanted, you could add a static method to MyImmutable builder to start from an existing MyImmutable instance:

public static MyImmutableBuilder basedOn(MyImmutable instance) {
  return new MyImmutableBuilder().foo(instance.foo).bar(instance.bar);
}

Answer 3

Your implementation is similar to the implementation detailed in Josh Bloch's Effective Java 2nd Edition.

One point of contention would be your build() method. If a single builder creates an immutable instance, would it be fair to allow the builder to be used again, considering that it's work has been done? The caution here is that even though you are creating an immutable object, the mutability of your builder may result in a few rather "surprising" bugs.

To correct this, it may be suggested that the build method should create the instance and then make the builder incapable of building the object again, requiring a new builder. Although the boiler-plate may seem tedious, the benefits to be reaped later outweigh the required effort at present. The implementation class can then receive a Builder instance as a constructor parameter, but the builder should then let the instance pull out all it's needed state, releasing the builder instance and preserving final references to the data in question.