Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

java nested builder pattern duplicate fields

Tags:

java

The nested builder patterns that I've come across online usually have something like this:

class Person{

    private int id;
    private String name;
    private int age;
    ... so on

    private Person(Builder builder){
        this.id = builder.id;
        this.name = builder.name;
        this.age = builder.age;
    }

    public static class Builder{

        private int id;
        private String name;
        private int age;
        ... so on

        public Builder id(int id){
            this.id = id;
            return this;
        }
        public Builder name(String name){
             this.name = name;
             return this;
        }
        .... so on

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

    }

}

My question is, is it necessary to duplicate fields in Person and Builder? It seems like a lot of redundant code. And my second question is, would the following code be a viable replacement, why or why not?

class Person{

    private int id;
    private String name;
    private int age;
    ... so on

    private Person(){}

    public static class Builder{

        private Person person = new Person();

        public Builder id(int id){
            this.person.id = id;
            return this;
        }
        public Builder name(String name){
             this.person.name = name;
             return this;
        }
        .... so on

        public Person build(){
            return person;
        }
        // UPDATED -- another build method
        public Person build(){
            Person built = this.person;
            this.person = new Person();
            return built;
        }

    }

}

Note: I understand this topic may be opinionated and there may not be a "right" answer, but I just want to hear different ideas and opinions. I'm not looking for the ultimate truth.

like image 981
user2914191 Avatar asked Aug 06 '17 21:08

user2914191


1 Answers

Your code would be fine as long as:

  1. you keep your Person member variables private (you are doing so)
  2. you don't provide methods that allow modification of those member variables (the code you show does not do, but you have omitted parts of it)
  3. those member variables are immutable or you ensure getters provide copies of them. usually better that the members are already immutable (hint: even java collections). otherwise you will be creating instances on each getX call.
  4. once Builder.build is called, noone must be able to modify Person instance state, not even Builder itself. this is not happening in the code you posted
  5. builder does not expose "temporal instance" being built (if any at all). No instance must be exposed aside the return of build method.

there are opinions about which is the preferred way or not, matter of taste most of the time. But in terms of being right or not, that approach would be fine with some modifications. At the end, what happens before the build is called is purely internal to the Builder. It's an implementation matter. The important thing is that the previous rules are met.

To fix rule 4: your Builder.build method should return a deep clone of the temp instance being used (there are ways to achcieve that without needing to specify each field). Or, you should have a flag in builder that forbids calling any other method on Builder instance, once build has been called.

Side note: i usually prefer that Builder class also uses private constructor. I would have this on Person class:

public static Builder builder() {
    return new Builder();
}

This can give you more flexibility on the way to initialize the Builder, or even you can have several builder methods doing not exactly the same stuff in terms of "preconfiguring" the builder (and since they are methods, you have more flexibility on naming than on constructors :) )

like image 188
albert_nil Avatar answered Nov 05 '22 16:11

albert_nil