Simple builder pattern implementation for building immutable objects

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

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

StackExchange Code Review Q#127391, answer score: 12