0

Recently I came into a situation where the builder pattern was very strong, but I had the need to subclass. I looked up some solutions and some suggested generics while others suggested normal subclassing. However, none of the examples I looked at had required fields in order to even begin building an object. I wrote a tiny example to illustrate where I'm getting stuck. At every turn I kept running into a wall of problems where things would return the wrong class, can't override static methods, returning super() returns the wrong data type, etc. I have a feeling there is no way out except excessive use of generics.

What is the correct way to go in this situation?

Tester

import person.Person;
import person.Student;

public class Tester
{
    public static void main(String[] args)
    {
        Person p = Person.builder("Jake", 18).interest("Soccer").build();
        // Student s = Student.builder(name, age) <-- It's weird that we still have access to pointless static method
        // Student s = Student.builder("Johnny", 24, "Harvard", 3).address("199 Harvard Lane") <-- returns Person builder, not student
        Student s = ((Student.Builder)Student.builder("Jack", 19, "NYU", 1).address("Dormitory")).build(); // really bad
    }
}

Person Class

package person;

import java.util.ArrayList;
import java.util.List;

public class Person
{
    // Required
    protected String name;
    protected int age;

    // Optional
    protected List<String> interests = new ArrayList<>();
    protected String address = "";

    protected Person(String name, int age)
    {
        this.name = name;
        this.age = age;
    }

    public String getName() { return name; }
    public int getAge() { return age; }
    public List<String> getInterests() { return interests; }
    public String getAddress() { return address; }

    // person.person does not allow builder construction
    // unless all required fields are provided

    /* Problem: I have to repeat the constructor fields here, very annoying */
    public static Builder builder(String name, int age)
    {
        Person p = new Person(name, age);
        return new Builder(p);
    }

    public static class Builder
    {
        Person reference;

        protected Builder(Person reference)
        {
            this.reference = reference;
        }

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

        public Builder interest(String interest)
        {
            reference.interests.add(interest);
            return this;
        }

        public Person build()
        {
            return reference;
        }
    }
}

Student Class

package person;

import java.util.ArrayList;
import java.util.List;

public class Student extends Person
{
    // Required
    protected String school;
    protected int year;

    // Optional
    protected List<String> subjects = new ArrayList<>();

    // This looks good
    public Student(final String name, final int age, final String school, final int year)
    {
        super(name, age);
        this.school = school;
        this.year = year;
    }

    public String getSchool() { return school; }
    public int getYear() { return year; }
    public List<String> getSubjects() { return subjects; }

    /* Here's where my issues are:

      * Override doesn't compile on static methods but how else can I describe that I want to
      * override this functionality from the Person class?
      * 
      * Extending 'Person' does not enforce that I need to provide 'name', 'age', etc like it would 
      * if this was a normal design pattern using the 'new' keyword. I have to manually drag fields
      * from 'person' and place them here. This would get VERY messy with an additional class 
      * 
      * User can STILL call the Person builder on a Student object, which makes no sense. */
    /*@Override*/ public static Builder builder(String name, int age, String school, int year)
    {
        Student s = new Student(name, age, school, year);
        return new Builder(s);
    }

    public static class Builder extends Person.Builder
    {
        // Student reference; <--- this should not be needed since we already
        //                      have a variable for this purpose from 'Person.Builder'

        public Builder(final Student reference)
        {
            super(reference);
        }

        /* Things begins to get very messy here */
        public Builder subject(String subject)
        {
            ((Student)reference).subjects.add(subject);
            // I guess I could replace the reference with a student one, but
            // I feel like that infringes on calling super() builder since we do the work twice.
            return this;
        }

        @Override public Student build()
        {
            // I can either cast here or
            // rewrite 'return reference' every time.
            // Seems to infringe a bit on subclassing.
            return (Student)super.build();
        }
    }
}
Hatefiend
  • 3,416
  • 6
  • 33
  • 74
  • The [builder doesn't have to be nested](https://stackoverflow.com/questions/19130876/builder-pattern-inside-vs-outside-class), but there would need to be some context to indicate which implementation to return. Keep the constructor for the base class, but if `setSchool()`, `setYear()`, or `addSubject()` is invoked, then return a Student instead of Person. – Andrew S Dec 04 '17 at 18:18
  • One of the values of the builder pattern is you don't end up with a bunch of constructor parameters. – DwB Dec 11 '17 at 17:03

2 Answers2

1

What you write here :

Student s = ((Student.Builder)Student.builder("Jack", 19, "NYU", 1).address("Dormitory")).build(); // really bad

is indeed not very natural and you should not need to cast.
We expect rather something like :

Student s = Student.builder("Jack", 19, "NYU", 1).address("Dormitory")).build(); 

Besides all casts you did in the implementation of Student.Builder are also noise and statements that may fail at runtime :

    /* Things begins to get very messy here */
    public Builder subject(String subject)   {
        ((Student)reference).subjects.add(subject);          
        return this;
    }

    @Override public Student build() {          
        return (Student)super.build();
    }

Your main issue is the coupling between the Builder classes and the building methods.
A important thing to consider is that at compile time, the method binding (method selected by the compiler) is performed according to the declared type of the target of the invocation and the declared type of its arguments.
The instantiated type is considered only at runtime as the dynamic binding is applied: invoking the method bounded at compile time on the runtime object.

So this overriding defined in Student.Builder is not enough :

@Override public Student build() {
    return (Student)super.build();
}

As you invoke :

Student.builder("Jack", 19, "NYU", 1).address("Dormitory").build();

At compile time, address("Dormitory") returns a variable typed as Person.Builder as the method is defined in Person.Builder :

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

and it not overriden in Student.Builder.
And at compile time, invoking build() on a variable declared as Person.Builder returns a object with as declared type a Person as the method is declared in Person.Builder as :

public Person build(){
    return reference;
}

Of course at runtime, the returned object will be a Student as

Student.builder("Jack", 19, "NYU", 1) creates under the hood a Student and not a Person.

To avoid cast to Student.builder both from the implementation and the client side, favor composition over inheritancy :

public static class Builder {

  Person.Builder personBuilder;
  private Student reference;

  public Builder(final Student reference) {
    this.reference = reference;
    personBuilder = new Person.Builder(reference);
  }

  public Builder subject(String subject) {
    reference.subjects.add(subject);
    return this;
  }

  // delegation to Person.Builder but return Student.Builder
  public Builder interest(String interest) {
    personBuilder.interest(interest);
    return this;
  }

  // delegation to Person.Builder but return Student.Builder
  public Builder address(String address) {
    personBuilder.address(address);
    return this;
  }

  public Student build() {
    return (Student) personBuilder.build();
  }

}

You can now write :

Student s = Student.builder("Jack", 19, "NYU", 1)
                   .address("Dormitory")
                   .build(); 

or even that :

Student s2 = Student.builder("Jack", 19, "NYU", 1)
                    .interest("Dance")
                    .address("Dormitory")
                    .build();

Composition introduces generally more code as inheritancy but it makes the code both more robust and adaptable.

As a side note, your actual issue is enough close to another question I answered 1 month ago.
The question and its answers may interest you.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
0

A few thoughts as background

  1. Static methods are not so great, they make unit testing more difficult.
  2. It is fine to put the builder as a static, nested class, but if you are using a builder to construct a class you should make the constructor not-public.
  3. I prefer to have the builder be a separate class in the same package and to make the constructor (of the class that is created by the builder) package access.
  4. Limit the builder constructor parameters.
  5. I'm not a fan of using a class hierarchy for builders.
  6. The Person and Student classes each have a builder.

Some Code

public class PersonBuilder
{
    private String address;
    private int age;
    private final List<String> interestList;
    private String name;

    public PersonBuilder()
    {
        interestList = new LinkedList<>();
    }

    public void addInterest(
        final String newValue)
    {
        // StringUtils is an apache utility.
        if (StringUtils.isNotBlank(newValue))
        {
            interestList.add(newValue);
        }

        return this;
    }

    public Person build()
    {
        // perform validation here.
        // check for required values: age and name.

        // send all parameters in the constructor.  it's not public, so that is fine.
        return new Person(address, age, interestList, name);
    }

    public PersonBuilder setAddress(
        final String newValue)
    {
        address = newValue;

        return this;
    }

    public PersonBuilder setAge(
        final int newValue)
    {
        age = newValue;

        return this;
    }

    public PersonBuilder setInterestList(
        final List<String> newValue)
    {
        interestList.clear();

        if (CollectionUtils.isNotEmpty(newValue))
        {
            interestList.addAll(newValue);
        }

        return this;
    }

    public PersonBuilder setName(
        final String newValue)
    {
        name = newValue;

        return this;
    }
}


public class Person
{
    private Person()
    {
    }

    Person(
        final String addressValue,
        final int ageValue,
        final List<String> interestListValue,
        final String name)
    {
        // set stuff.
        // handle null for optional parameters.
    }

    // create gets or the fields, but do not create sets.  Only the builder can set values in the class.
}
DwB
  • 37,124
  • 11
  • 56
  • 82