Simple builder pattern implementation for building immutable objects

Question

This is a builder pattern implementation to build immutable Person objects:

Person class

public final class Person {

    // All are set to final to make Person Immutable
    private final int id;
    private final String name;
    private final String surname;
    private final boolean isOccupied;

    // Constructor is private, so that only static
    // PersonBuilder can initiate the Person class instance
    private Person(PersonBuilder builder) {
        this.id = builder.getId();
        this.name = builder.getName();
        this.surname = builder.getSurname();
        this.isOccupied = builder.isOccupied();
    }

    public static class PersonBuilder {

        // Member variables of PersonBuilder are temporary storage
        // to create Immutable Person class instance
        private int id;
        private String name;
        private String surname;
        private boolean isOccupied;

        PersonBuilder() { }

        // The only method to initiate Person class
        Person build() {
            return new Person(this);
        }

        // Multiple Constructors for each member variable
        public PersonBuilder id(int id) {
            this.id = id;
            return this;
        }

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

        public PersonBuilder surname(String surname) {
            this.surname = surname;
            return this;
        }

        public PersonBuilder setOccupied(boolean isOccupied) {
            this.isOccupied = isOccupied;
            return this;
        }

        // getters, these will be used in private constructor
        // of Person class
        public int getId() {
            return id;
        }

        public String getName() {
            return name;
        }

        public String getSurname() {
            return surname;
        }

        public boolean isOccupied() {
            return isOccupied;
        }
    }

    // getters
    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public String getSurname() {
        return surname;
    }

    public boolean isOccupied() {
        return isOccupied;
    }

    @Override
    public String toString() {  
        return String.format(
                "Id:\t\t%d\nName:\t\t%s\nSurname:\t%s\nIsOccupied:\t%s\n"
                , id, name, surname, isOccupied);
    }

}

Person demo

public class PersonDemo {

    public static void main(String[] args) {
        // PersonBuilder instance
        Person.PersonBuilder builder = new Person.PersonBuilder();

        // builder builds the instance
        builder.name("Levent").surname("Divilioglu").id(666).setOccupied(true);

        // Immutable Person object is initiated with build() method
        Person person = builder.build();

        System.out.println(person);
    }

}

Demo output

Id:           666
Name:     Levent
Surname:  Divilioglu
IsOccupied:   true

I did write this implementation to learn the GoF builder pattern. Immutability is important for thread-safety, thus set the class and it's members final. I also set the Person constructor private to guarantee that it is only instantiated with inner class method, and only once. On each build() method, a new unique Person instance will be created.

I'm aware that builder patter is used for classes that have a lot of many member variables but I did only used 4 member variables for the simplicity.

Answer 1

First of all, very good job.

Now I will point out a few things that I think can be improved on.

Code Duplication

You have copied over all the private fields from Person to the Builder, which is necessary when the fields are declared final. As coderodde pointed out, using a final declaration is not necessary here when the fields are private and there are no setters.

Now that the fields are no longer declared final, it is possible to hold an unfinished version of your Person object in your builder. Now all the fields from Person no longer need to be copied over.

This also prevents the need for the getter methods on your Builder and the weird constructor of Person that takes a Builder as a parameter. The Person class should not be initializing itself via the state of the Builder object. The Builder should be building the Person, hence the name of the pattern.

Excessive commenting

You use comments a lot, even at places where they aren't needed and are generally just noisy. For example:

// getters

and

// getters, these will be used in private constructor
// of Person class`.

The first one is simply redundant, since all the methods below it are called get..., clearly indicating a getter function. The second one also provides little useful information. Note that generally, someone will read a code file from top to bottom, unless they might be looking for the details on a specific method. This would mean that they have already scrolled past your constructor and seen the getters being used there.

But there is a bigger point I want to make here with regards to the comments. In general, using comments to express what your code does, means that you have failed to express yourself in code, which can document itself when names are chosen carefully.

Coming back to the second comment I mentioned. You probably wrote that because you felt that you had to explain yourself for having public getters on your Builder class. When placing a comment like this, take a step back and ask yourself why you placed that comment and if your code would still seem logical without it. If your code seems weird without the comment, which I think public getters on a Builder class do, then you need to fix that issue instead of commenting about why you did it.

Now by moving the initialization of the Person to the Builder class where it belongs, as mentioned above, the Builder no longer needs these methods and the comment can be removed.

The same goes for the comment about the temporary member variables, which can now be removed as the Builder simply holds an instance of the Person class, which makes perfect sense on its own.

I went on a bit of a rant about comments, but I hope you can see what I mean.

Naming

Overall, job well done with the naming. A few remarks though:

• I'd rename PersonBuilder to Builder. It's an inner class of Person so there really is no need for it. Person.Builder is perfectly fine.
• I'd also rename the individual building methods for id, name, surname and isOccupied to setId, setName, setSurname and setOccupied, since they are all effectively setters.

Reworked Code

When incorporating these points into your code, it should look a little something like this.

public final class Person {
    private int id;
    private String name;
    private String surname;
    private boolean isOccupied;

    private Person() {}

    public static class Builder {
        private Person personToBuild;

        Builder() {
            personToBuild = new Person();
        }

        Person build() {
            Person builtPerson = personToBuild;
            personToBuild = new Person();

            return builtPerson;
        }

        public Builder setId(int id) {
            this.personToBuild.id = id;
            return this;
        }

        public Builder setName(String name) {
            this.personToBuild.name = name;
            return this;
        }

        public Builder setSurname(String surname) {
            this.personToBuild.surname = surname;
            return this;
        }

        public Builder setOccupied(boolean isOccupied) {
            this.personToBuild.isOccupied = isOccupied;
            return this;
        }
    }

    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public String getSurname() {
        return surname;
    }

    public boolean isOccupied() {
        return isOccupied;
    }

    @Override
    public String toString() {  
        return String.format(
                "Id:\t\t%d\nName:\t\t%s\nSurname:\t%s\nIsOccupied:\t%s\n"
                , id, name, surname, isOccupied);
    }
}

Answer 2

You can simplify your builder by removing the final specifiers from the fields of Person. Don't worry, unless you add setters for the person type, it is effectively immutable.

toString() is not usually used as a method for printing human-readable output, but rather used as an aid in debugging.

I had this in mind:

public final class Person {

    // The class has no setters, so it's effectively immutable. We need a 
    // possibility of setting to these fields so that we can construct them in 
    // the builder.
    private int id;
    private String name;
    private String surname;
    private boolean isOccupied;

    // Hide the constructor from the user.
    private Person() {}

    public static final class Builder {

        private Person person;

        // Initialize state.
        private Builder(final Person person) {
            this.person = person;
        }

        // Start building a person.
        public static Builder build() {
            return new Builder(new Person());
        }

        public Builder id(final int id) {
            this.person.id = id;
            return this;
        }

        public Builder name(final String name) {
            this.person.name = name;
            return this;
        }

        public Builder surname(final String surname) {
            this.person.surname = surname;
            return this;
        }

        public Builder occupied(final boolean isOccupied) {
            this.person.isOccupied = isOccupied;
            return this;
        }

        // End construction and return a unique person object.
        public Person get() {
            final Person ret = new Person();
            ret.id         = person.id;
            ret.name       = person.name;
            ret.surname    = person.surname;
            ret.isOccupied = person.isOccupied;
            return ret;
        }
    }

    // getters
    public int getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public String getSurname() {
        return surname;
    }

    public boolean isOccupied() {
        return isOccupied;
    }

    @Override
    public String toString() {  
        return String.format("[id: %d, name: %s, surname: %s, occupied: %b]",
                             id, 
                             processNull(name),
                             processNull(surname),
                             isOccupied);
    }

    private static String processNull(final String str) {
        return str == null ? "null" : "\"" + str + "\"";
    }
}

public class PersonDemo {

    public static void main(String[] args) {
        Person person1 = Person.Builder
                               .build()
                               .name("Funky")
                               .surname("Funk")
                               .id(1337)
                               .occupied(true)
                               .get();

        Person person2 = Person.Builder
                               .build()
                               .name("Alice")
                               .id(123)
                               .get();

        System.out.println(person1);
        System.out.println(person2);
    }
}

Hope that helps.

Answer 3

If you would use Lombok, you can get the same classes with less code:

@Value
@Builder
public class Person {
    int id;
    String name;
    String surname;
    boolean isOccupied
}

In Value types, all fields are private and final by default, and the class is also final. Getters, equals, hashCode and toString are also generated.

The Builder annotation will generate a private all-args constructor, a nested builder class PersonBuilder, and a public static PersonBuilder builder() in Person.

Disclosure: I am a lombok developer.

Answer 4

Several people are recommending to remove the final keyword from the private fields. I would strongly recommend against doing that. The whole point of the final keyword is to prevent accidental re-assignments to these fields from other methods in the class (private and public). So, if a Person is immutable, then all the fields in Person should be marked final. I actually have my IDE configured to automatically mark fields private on save. Immutability is a good thing to have unless you really want mutable state.

Your builder does not need getters; remove them. I would not inject the builder into the Person constructor and keep the business of building in the builder. Instead inject the individual fields from the build method. This way, you can make the Person constructor public and use it without the builder as well and you stick to the principle that constructors should not do work (other than assigning arguments).

Now regarding the getters on Person; if your fields are final, you might as well make them public and skip adding getter methods. Why type myobject.getFoo() when you can just type myobject.foo? The only reason to have getters at all is that is if you are depending on frameworks that still expect javabeans style getters with the naming conventions and verbose sillyness that comes with that. If that's not the case, don't add them.

Some other things I would tweak are moving the Builder.build() method to public static Builder Person.person(){...}. This will simplify the business of creating a Person builder and you won't need to type Person.Builder.build() which looks a bit ugly to me.