Jason Fotinatos
Jason Fotinatos

Reputation: 281

Improvements to Joshua Bloch's Builder Design Pattern?

Back in 2007, I read an article about Joshua Blochs take on the "builder pattern" and how it could be modified to improve the overuse of constructors and setters, especially when an object has a large number of properties, most of which are optional. A brief summary of this design pattern is articled here [http://rwhansen.blogspot.com/2007/07/theres-builder-pattern-that-joshua.html].

I liked the idea, and have been using it since. The problem with it, while it is very clean and nice to use from the client perspective, implementing it can be a pain in the bum! There are so many different places in the object where a single property is reference, and thus creating the object, and adding a new property takes a lot of time.

So...I had an idea. First, an example object in Joshua Bloch's style:

Josh Bloch Style:

public class OptionsJoshBlochStyle {

    private final String option1;
    private final int option2;
    // ...other options here  <<<<

    public String getOption1() {
        return option1;
    }

    public int getOption2() {
        return option2;
    }

    public static class Builder {

        private String option1;
        private int option2;
        // other options here <<<<<

        public Builder() {
        }

        public Builder option1(String option1) {
            this.option1 = option1;
            return this;
        }

        public Builder option2(int option2) {
            this.option2 = option2;
            return this;
        }

        public OptionsJoshBlochStyle build() {
            return new OptionsJoshBlochStyle(this);
        }
    }

    private OptionsJoshBlochStyle(Builder builder) {
        this.option1 = builder.option1;
        this.option2 = builder.option2;
        // other options here <<<<<<
    }

    public static void main(String[] args) {
        OptionsJoshBlochStyle optionsVariation1 = new OptionsJoshBlochStyle.Builder().option1("firefox").option2(1).build();
        OptionsJoshBlochStyle optionsVariation2 = new OptionsJoshBlochStyle.Builder().option1("chrome").option2(2).build();
    }
}

Now my "improved" version:

public class Options {

    private String option1;
    private int option2;
    // ...other options here

    public String getOption1() {
        return option1;
    }

    public int getOption2() {
        return option2;
    }

    public static class Builder {

        private final Options options;

        public Builder() {
            this.options = new Options();
        }

        public Builder option1(String option1) {
            this.options.option1 = option1;
            return this;
        }

        public Builder option2(int option2) {
            this.options.option2 = option2;
            return this;
        }

        public Options build() {
            return options;//new RequestContext(this);
        }
    }

    private Options() {

    }

    public static void main(String[] args) {
        Options optionsVariation1 = new Options.Builder().option1("firefox").option2(1).build();
        Options optionsVariation2 = new Options.Builder().option1("chrome").option2(2).build();

    }
}

As you can see in my "improved version", there are 2 less places in which we need to add code about any addition properties (or options, in this case)! The only negative that I can see is that the instance variables of the outer class are not able to be final. But, the class is still immutable without this.

Is there really any downside to this improvement in maintainability? There has to be a reason which he repeated the properties within the nested class that I'm not seeing?

PS: Not sure if this is an appropriate type of question for StackOverflow or belongs on something more subjective like [programmers.stackexchange.com], so I apologize in advance!

EDIT 1:

@irreputable - Something like this in Java? I still don't see how this becomes thread safe, because of the change. I'll have to look into safe publishing, as you suggest.

public class OptionsDelegate {

    private final OptionsData data;

    private static class OptionsData {
         String option1;
         int option2;
    }

    // ...other options here

    public String getOption1() {
        return data.option1;
    }

    public int getOption2() {
        return data.option2;
    }

    public static class Builder {

        private OptionsData data;

        public Builder() {
            this.data = new OptionsData();
        }

        public Builder option1(String option1) {
            this.data.option1 = option1;
            return this;
        }

        public Builder option2(int option2) {
            this.data.option2 = option2;
            return this;
        }

        public OptionsDelegate build() {
            OptionsDelegate optionsDelegate = new OptionsDelegate(this.data);
            this.data = null;
            return optionsDelegate;
        }
    }

    private OptionsDelegate(OptionsData data) {
        this.data = data;
    }

    public static void main(String[] args) {
        OptionsDelegate optionsVariation1 = new OptionsDelegate.Builder().option1("firefox").option2(1).build();
        OptionsDelegate optionsVariation2 = new OptionsDelegate.Builder().option1("chrome").option2(2).build();


    }
}

Upvotes: 4

Views: 4088

Answers (4)

marcocast
marcocast

Reputation: 401

Have a look at the step builder pattern if you want to give a better user experience, but still keep your code clean.

Upvotes: 0

mulya
mulya

Reputation: 1331

You can't use your version (first one) in case when class Options can't be instantiated in not fully initialized state, for example when constructor of Options contains validation of fields (option1, option2). So original version of Builder is more flexible. And as mentioned above your second version of Builder create mutable object.

Upvotes: 0

irreputable
irreputable

Reputation: 45443

final guarantees "safe publishing". your version is not thread safe. The premise of this thing is how to create objects with final fields in a friendlier way, removing final defeats that goal.

This is possible though:

class Options

    class Data
        option1;
        option2;        

    final Data data;
    Options(Data data){ this.data=data; }

    getOption1(){ return data.option1; }
    getOption2(){ return data.option2; }

    class Builder

        Data data = new Data();

        setOption1(op1){ data.option1=op1; }
        setOption2(op2){ data.option2=op2; }

        Options build()
        { 
            Options o = new Options(data); 
            data = null;
            return o;
        }

Upvotes: 5

Jon Skeet
Jon Skeet

Reputation: 1502066

(I'm expecting this question to be migrated, but I'll answer anyway - the answer will be preserved.)

You're claiming the class is still immutable... but I don't think it is.

Options.Builder builder = new Options.Builder().option1("foo").option2(1);
Options options = builder.build();

builder.option1("changed");
System.out.println(options.getOption1());

Note that with a bit of a modification to prevent this (set options to null in the build() method so that the builder can't be reused) this is basically the pattern that the Java implementation of Protocol Buffers used to use. I believe it now uses something closer to Josh's earlier pattern.

Upvotes: 12

Related Questions