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

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 2

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.