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.

• coderodde: The builder object maintains a state. If you call build() several times in a row, you will create as much identical persons. Is that your intention?
• Levent Divilioglu: It is a builder pattern structure to use flexible dot notation however each instance of Person will be unique, thus immutable. If build() called several times inside a Person constructor, each Person object will be immutable and unique, that was my intention. And the Person objects thread-safe as all member variables are final, are settable only for once.

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);
 }
}

• Wayman Bell III: I agree with many of your points concerning comments, but I must respectfully disagree with your comment that explaining code function in a comment means the coding is done poorly. Generally speaking, programmers come with different levels of experience, knowledge, and expertise. Knowed programmers might not need the documentation to understand the code's function, but novice programmers very well may.
• Levent Divilioglu: @Thijs Riezebeek : Thanks for your useful comments and your demo code. I'll check out internet and some books in order to check your code with other sources but I must point that I've found it very useful. Important thing is to know where to use which approach and I want to be aware of every approaches. I also agree that commenting is important to guide&notify other programmers and for documentation considerations. Thanks.
• Eric Guan: Can you explain the thought process behind Builder.build()? Why are you creating a new Person reference and re-instantiating personToBuild, instead of simply returning personToBuild? Is it just for clarity of semantic meaning?
• Thijs Riezebeek: @Eric Guan, the reason for that is to reset the builder. Once you call the build() method, it is common practice to reset the state of the builder so it can be reused.

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.

• Levent Divilioglu: Thanks for toString() comment, I agree but why not use final keyword? If I used this code in a big project with a team, I would prefer anyone to ask himself/herself that why a final keyword is used so that he/she could foresee that it's used for the sake of object state consistency. I also wonder that if is there any trade-off for using and not using final keyword.
• bowmore: Removing final from the Person fields does mean that some visibility guarantees are lost, from a multi threading point of view.
• Levent Divilioglu: @bowmore Can you tell & explain a little bit more about usage of final for the point of thread-safety ?
• bowmore: @LeventDivilioglu "Final fields can’t be modified (although the objects they refer to can be modified if they are mutable), but they also have special semantics under the Java Memory Model." More specifically, an object with only final fields can be safely published and shared, through any mechanism. Without the final modifiers the same Object must be safely published (but can be shared without further synchronization). So omitting final on its fields would make Person, in this question, an object we should make sure to publish safely. (see Java Concurrency in Practice for a full explanation)
• bowmore: A mutable object's fields cannot all be final. If you define a class whose instances will be shared across threads, then making it immutable and have final fields is the best option (if that is possible). But even a mutable object can be shared between threads, yet you'll need explicit synchronization to pull that off. Some objects are not meant to be shared across threads, and all of this is not even a concern. Encapsulation is always a good design choice; if you can do without an accessor, leave it out. If you can make a field final, make it final.

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.

• Levent Divilioglu: After years, have seen this comment. If I was going to write the same code now, definitely agree and apply what you have written. I don't know why I have written getters inside the builder :)
• Jilles van Gurp: I moved on and now use Kotlin and data classes. Kotlin has no need for builders as it has default immutable, final vals for parameters, and a few other nice things. With Java, you now have records but they are still mutable by default. And people also use lombok of course, which generates all this cruft for you. Never was a big fan of that but it's a valid option.
• Jilles van Gurp: For reference, this is all you need in Kotlin. Immutable, minimal, etc. And you get usable toString, hashCode, equals, etc. kotlin data class Person(val id: Long, val name: String, val surName: String?=null, val isOccupied: Boolean = false) val p1=Person(1,"Alice") val p2=Person(2,"Bob", "Alison",true)