Is this Monster Builder a good Builder / Factory pattern for abstracting long constructors mixed with setters?

Question

This is a human interface question about combining the step builder pattern with the enhanced or wizard builder patterns into a creational DSL. It uses a fluent like interface, although it uses method chaining, not cascading. That is, the methods return differing types.

I’m confronting a monster class that has two constructors that take a mix of ints, Strings, and an array of Strings. Each constructor is 10 parameters long. It also has about 40 optional setters; a few of which conflict with each other if used together. Its construction code looks something like this:

Person person = Person("Homer","Jay", "Simpson","Homie", null, "black", "brown", 
  new Date(1), 3, "Homer Thompson", "Pie Man", "Max Power", "El Homo", 
  "Thad Supersperm", "Bald Mommy", "Rock Strongo", "Lance Uppercut", "Mr. Plow");

person.setClothing("Pants!!");     
person.setFavoriteBeer("Duff");
person.setJobTitle("Safety Inspector");

This eventually fails because it turns out having set both favorite beer and job title is incompatible. Sigh.

Redesigning the monster class is not an option. It’s widely used. It works. I just don’t want watch it being constructed directly any more. I want to write something clean that will feed it. Something that will follow its rules without making developers memorize them.

Contrary to the wonderful builder patterns I’ve been studying this thing doesn’t come in flavors or categories. It demands some fields all the time and other fields when needed and some only depending on what has been set before. The constructors are not telescoping. They provide two alternate ways to get the class into the same state. They are long and ugly. What they want fed to them varies independently.

A fluent builder would definitely make the long constructors easier to look at. However, the massive number of optional setters clutters the required ones. And there is a requirement that a cascading fluent builder doesn’t satisfy: compile time enforcement.

Constructors force the developer to explicitly add required fields, even if nulling them. This is lost when using a cascading fluent builder. The same way it's lost with setters. I want a way to keep the developer from building until each required field has been added.

Unlike many builder patterns, what I’m after isn’t immutability. I’m leaving the class as I found it. I want to know the constructed object is in a good state just by looking at the code that builds it. Without having to refer to documentation. This means it needs to take the programmer thru conditionally required steps.

Person makeHomer(PersonBuilder personBuilder){ //Injection avoids hardcoding implementation
    return personBuilder

         // -- These have good default values, may be skipped, and don't conflict -- //
        .doOptional()
            .addClothing("Pants!!")   //Could also call addTattoo() and 36 others

         // -- All fields that always must be set.  @NotNull might be handy. -- //
        .doRequired()                 //Forced to call the following in order
            .addFirstName("Homer")
            .addMiddleName("Jay")
            .addLastName("Simpson")
            .addNickName("Homie")
            .addMaidenName(null)      //Forced to explicitly set null, a good thing
            .addEyeColor("black")
            .addHairColor("brown")
            .addDateOfBirth(new Date(1))
            .addAliases(
                "Homer Thompson",
                "Pie Man",
                "Max Power",
                "El Homo",
                "Thad Supersperm",
                "Bald Mommy",
                "Rock Strongo",
                "Lance Uppercut",
                "Mr. Plow")

         // -- Controls alternatives for setters and the choice of constructors -- //
        .doAlternatives()           //Either x or y. a, b, or c. etc.
            .addBeersToday(3)       //Now can't call addHowDrunk("Hammered"); 
            .addFavoriteBeer("Duff")//Now can’t call addJobTitle("Safety Inspector");  

        .doBuild()                  //Not available until now
    ;
}   

Person may be built after addBeersToday() since at that point all constructor info is known but is not returned until doBuild().

public Person(String firstName, String middleName, String lastName,
               String nickName, String maidenName, String eyeColor, 
               String hairColor, Date dateOfBirth, int beersToday, 
               String[] aliases);

public Person(String firstName, String middleName, String lastName,
               String nickName, String maidenName, String eyeColor, 
               String hairColor, Date dateOfBirth, String howDrunk,
               String[] aliases);

These parameters set fields that must never be left with default values. beersToday and howDrunk set the same field different ways. favoriteBeer and jobTitle are different fields but cause conflicts with how the class is used so only one should be set. They are handled with setters not constructors.

The doBuild() method returns a Person object. It's the only one that does and Person is the only type it will return. When it does Person is fully initialized.

At each step of the interface the type returned is not always the same. Changing the type is how the developer is guided though the steps. It only offers valid methods. The doBuild() method isn’t available until all needed steps have been completed.

The do/add prefixes are a kludge to make writing easier because the changing return type mismatches with the assignment and makes intelisense recommendations become alphabetical in eclipse. I've confirmed intellij doesn't have this problem. Thanks NimChimpsky.

This question is about the interface, so I'll accept answers that don't provide an implementation. But if you know of one, please share.

If you suggest an alternative pattern please show it's interface being used. Use all the inputs from the example.

If you suggest using the interface presented here, or some slight variation, please defend it from criticisms like this.

What I really want to know is if most people would prefer to use this interface to build or some other. This is human interface question. Does this violate PoLA? Don't worry about how hard it would be to implement.

However, if you are curious about implementations:

A failed attempt (didn't have enough states or understand valid vs not defaulted)

A step builder implementation (not flexible enough for multiple constructors or alternatives)

An enhanced builder (Still liner but has flexible states)

Wizard builder (Deals with forking but not remembering the path to pick a constructor)

Requirement:

• The monster (person) class is already closed to modification and extension; no touchy

Goals:

• Hide the long constructors since the monster class has 10 required parameters
• Determine which constructor to call based on alternatives used
• Disallow conflicting setters
• Enforce rules at compile time

Intent:

• Clearly signal when default values are not acceptable

Answer 1

a static inner builder, made famous by josh bloch in effective java.

Required parameters are constructor args, optional are methods.

An example. The invocation where only username is required :

RegisterUserDto myDto = RegisterUserDto.Builder(myUsername).password(mypassword).email(myemail).Build();

And the underlying code(omitting the obvious instance vars) :

private RegisterUserDTO(final Builder builder) {
        super();
        this.username = builder.username;
        this.firstName = builder.firstName;
        this.surname = builder.surname;
        this.password = builder.password;
        this.confirmPassword = builder.confirmPassword;
    }


    public static class Builder {
        private final String username;

        private String firstName;

        private String surname;

        private String password;

        private String confirmPassword;

        public Builder(final String username) {
            super();
            this.username = username;
        }

        public Builder firstname(final String firstName) {
            this.firstName = firstName;
            return this;
        }

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

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

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

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

Answer 2

So the static inner builder combined with a factory function can do some of what you want. (1) It can enforce dependencies of the type if A is set, also B. (2) It can return different classes. (3) It can do logic checking on the entries.

However, it will still fail if the programmers enters the wrong fields.

A possible advantage is the "multiple builders" pattern. If the client knows ahead of time the purpose of why he is building a particular element, then he can get a different builder. You can make a builder for every combination.

Depending on the type of logical dependencies in your class you can combine these multiple builders with one general builder. You can have, say, a general builder, and when you call setOption(A) on the general builder it returns a different class of builder, to which you can only chain the methods that continue to be relevant.Thus you get fluency, but you can exclude some paths. When you do this you have to be careful to nullify fields which were set but have become irrelevant - you cannot make the builders sub classes of each other.

This can force the client to choose at compile time how to construct the object, is that what you are after?

UPDATE - tried to answer the comments:

So first things first - a factory function is item one in Joshua Blocks Effective Java, it just means that for an object you make the constructor private, and instead make a static factory function. This is more flexible than a constructor, because it can return different object types. When you combine a factory function with multiple builders you can get a very powerful combination indeed. Here is the pattern description: http://en.wikipedia.org/wiki/Factory_method_pattern http://en.wikipedia.org/wiki/Abstract_factory_pattern

So imagine that you want to create objects that describe a person and their job, but when you specify their job you want to have a list of specific sub options.

public class outer{
    Map <Map<Class<?>, Object> jobsMap - holds a class object of the new builder class, and a prototypical isntance which can be cloned.
    outer(){
        jobsMap.add(Mechanic.class, new Mechanic());
        //add other classes to the map
    }

    public class GeneralBuilder{
    String name;
    int age;

//constructor enforces mandatory parameters.
    GeneralBuilder(String name, int age, \\other mandatory paramerters){
        //set params
    }

    public T setJob(Class<T extends AbstractJob> job){
        AbstractJob result = super.jobsMap.getValue(job).clone();
        //set General Builder parameters name, age, etc
        return (T) result;
    }
}

public MechanicBuilder extends AbstractJobs{
//specific properties
    MechanicBuilder(GeneralBuilder){
      // set age, name, other General Builder properties
    }
    //setters for specific properties return this
    public Person build(){
        //check mechanic mandatory parameters filled, else throw exception.
        return Person.factory(this);
    }
}

public abstract class AbstractJob{
    String name;
    String age;
    //setters;
}

public class Person {
//a big class with lots of options
//private constructor
    public static Person Factory(Mechanic mechanic){
        //set relevant person options
    }
}

So this is now fluent. I create an instance of outer, and fill the map with all the specific jobs types. I then can create from this as many new builders as I want as instances of the inner class. I set the parameters for the general builder calling .setJobs(Mechanic.class) and it returns an instance of mechanic which has a bunch of specific properties, which i can now call fluently using .setOptionA() etc. Eventually I call build, and this calls the static factory method in the Person class and passes itself. You get back a person class.

Its a lot of implementation, as you have to create a specific builder class for each "type" of object that might be represented by the Person class, but it does make an API that is very easy for the client to use. In practice, although these classes have lots of options, in practice there may be only a handful of patterns that people intend to create, and all the rest only appear by accident.

Answer 3

Instead of your builder pattern I suggest creating a new constructor that introduces different parameter objects, which group different parameters. Then you can delegate to the original constructor from within that new constructor. Also mark the original constructor as deprecated and point to the new one.

Refactoring to the constructor with parameter objects can also be done with IDE support and is therefore not much work. This way you could also refactor existing code. Than you still can create builders for the parameter objects and the class in question if they are still needed.

The problem you need to focus on is that different parameters depend on each other. And such dependencies should be reflected in objects of their own.

The problem with the chained builder is that you need just too much classes and that you can't change the order you are going to use them, even though that order is still correct.

Answer 4

So I have had several years to think about this, and I think I now know the right way to so this.

Firstly: implement every mandatory setting as a single method interface, which returns the next single method interface. This forces the client to fill out all required parameters, and has the added benefit that they must all be filled out in the same order everywhere in the code, making it easier to spot bugs.

Secondly: implement all stand alone optional parameters in one interface which is the return type of the final parameter.

Thirdly: for any complicated sub-group of optional parameters, make more interfaces which force a choice of routes to go down.

interface FirstName {

     public LastName setFirstName(String name)

}
interface LastName {
    public OptionalInterface setLastName(String name)
}

interface OptionalParams {
    public OptionalParams setOptionalParam(String numberOfBananas)
    public OptionalParams setOptionalParam(int numberOfApples)
    public AlcoholLevel setAlcoholLevel() // go down to a new interface
    public MaritalStatus setMaritalStatus()

    public Person build()
}

interface AlcoholLevel {
    //mutually exclusive options hoist you back to options
    public OptionalParams setNumberOfBeers(int bottles)
    public OptionalParams setBottlesOfWine(int wine)
    public OptionalParams setShots(int shots)
    public OptionalParams isHammered()
    public OptionalParams isTeeTotal()
    public OptionalParams isTipsy()
}

interface MaritalStatus {
    public OptionalParams isUnmarried()
    //force you into a required params loop
    public HusbandFirstName hasHusband()
    public WifeFirstName hasWife()
}

By having a series of one method interfaces you can largely force good behaviour on the client. This pattern works well with, for example, forming well formed HTTP requests in networks where certain authentications are required. An overlay of interfaces on top of a standard httml library steers clients in the right direction.

Some conditional logic is basically too hard to be worth it. Things like insisting that the sum of parameter 1 and parameter 2 are less than parameter 3 are best dealt with by throwing a runtime exception on the build method.

Answer 5

Disclaimer

I know the question is tagged java, however it states multiple times that it is a "human interface question", so I think that a sketch of a solution in another language like C++ can be useful, even if just for readers like me that end up here exploring design patterns (and questioning whether learning them is useful or a creativity stopper) with a language-agnostic approach.

Type safety

Given the constructors that you're forced to accept, I would say that a builder pattern can't do miracles, in the sense that, even if a constructor with all those parameters is ugly (I agree with you), trading it for a long chain of methodCalls(withArgs) in makeHomer doesn't make a tremendous improvement, imho; and that's unavoidable. After all, you still have to pass quite a few parameters, one way or another.

Therefore I think it can be productive to step back and reason again about what exactly is wrong with the constructor, in an attempt to slim down the solution you've already devised. We certainly have to accept that the solution will not be a short one-liner, but maybe we can improve it.

Indeed, the pivotal point of the solution I propose (alas, in C++ not in Java) is that the problem with that constructor is not (or not only) that it takes 10 params, but that you can mistakenly mix them up entirely and still get an (invalid) person.

After all, you already do that with your design by "converting" something which the compiler can't use to enforce anything (parameters' names) into something that it can use in that respect (memeber function names).

So all you want is a way to call those two already existing constructors, but at the same time reducing to zero the chance of entering parameters in the wrong order (or not entering the required ones, or entering incompatible ones, or whatever) by making the compiler error out everytime you make such a mistake.

Incidentally, and quite welcome is that a good IDE would accordingly produce

• good diagnostics,
• as well as inline suggestions

(The above screenshots show my IDE (Vim) in action, when I've made a mistake and when I'm in the middle of passing the arguments to makePerson.)

An alternative to using member function names to encode the order/optionality/incompatibility of the parameters is to create a type-safe wrapper around the constructors by associating an ad-hoc type to each of the arguments. Once you do so, you can just call the constructor wrapper with object of the types its signature enforces.

In C++, user defined literals also help you write clear code in this respect. For instance, if you define these:

struct FirstName {
  std::string str;
};
FirstName operator"" _first(char const* s, std::size_t) {
  return {s};
}

then you can write "Homer"_first to create an object of class FirstName.

Avoid invalid states

These lines

         // -- Controls alternatives for setters and the choice of constructors -- //
        .doAlternatives()           //Either x or y. a, b, or c. etc.
            .addBeersToday(3)       //Now can't call addHowDrunk("Hammered"); 
            .addFavoriteBeer("Duff")//Now can’t call addJobTitle("Safety Inspector");  

(maybe also in view of the word Now in your comments) suggest that you look at this logic as one which handles some time-varying states and that checks that something has happened already (e.g. calling addBeersToday) to prevent you from doing something else later (e.g. calling addHowDrunk). A probably better approach is to just avoid explictly dealing with such states. After all, the method chaining you refer to, does just that: it leverages compile-time information (the types) to prevent you from even trying to call addHowDrunk after addBeersToday.

But in C++, with function overloading you can do just that. And if the various overloads have much in common, you can actually write all of them at once, using if constexpr to branch on the few conditions that make them different.

The proposed approach

The solution I propose below, allows you to write makeHomer like this:

Person makeHomer() {
    return makePerson(
            "Homer"_first,
            "Jay"_middle,
            "Simpson"_last,
            std::nullopt,
            May/12/1956,
            {"Homer Thompson"_alias, "Max Power"_alias},
            3_beers,
            "Duff"_favBeer)
        .setClothing("Pants!!");
}

where makePerson is playing roughly the role of the builder or, to be more precise, the role of a type-safe Person's constructor. The advantage of relying on strong types is that, for instance, you can't even swap "Homer"_first and "Jay"_middle in the call above, otherwise you get a compile time error.

Here's another possible usage of makePerson:

Person makeMaude() {
    return makePerson(
            "Maude"_first,
            ""_middle,
            "Flanders"_last,
            "???"_maiden,
            May/12/1956,
            {},
            "teetotal"_drunk /* instead of 3_beers */,
            "Ned's wife"_job /* instead of "Duff"_favBeer */)
        //.setClothing("Pants!!") let's leave her naked
        ;

where you can see that I'm passing arguments of the alternative types to the last two parameters.

The function makePerson does no more than unwrapping the strings from within the strongly typed wrappers and forwarding them to one or the other constructor based on a compile-time conditional:

template<Drunkness Drunkness_, BeerOrJob BeerOrJob_>
Person makePerson(
        FirstName first,
        MiddleName middle,
        LastName last,
        std::optional<MaidenName> maiden,
        Date birth,
        std::vector<Alias> aliases,
        Drunkness_ drunkness,
        BeerOrJob_ /* beerOrJob */) {

    // Not sure how to use beerOrJob... Maybe by creating the Person but not
    // returning it before calling a setter similar to setClothing?
    if constexpr (std::is_same_v<Drunkness_, BeersToday>) {
        return Person(first.str, middle.str, last.str,
                maiden.value_or("null"_maiden).str, birth,  drunkness.num,
                aliases | transform(to_string) | to_vector);
    } else {
        return Person(first.str, middle.str, last.str,
                maiden.value_or("null"_maiden).str, birth,  drunkness.str,
                aliases | transform(to_string) | to_vector);
    }
}

Notice that I'm using concept Drunkness to express that an object of only one of two types can be passed to that argument (and similarly for BeerOrJob): I'm basically collapsing 4 overloads in a single one via templates and if constexpr.

---

Here's an example on Compiler Explorer, and here are a few comments about it:

• I've removed eye color and hair color only to shorten the exmaple, because they pose the same exact challenge as first name, last name, and others (similarly to how you've shown only one optional parameter, the clothing, commenting that there could be many more).
• I enforce type safety as much as I can, but as little as it's needed: all those types that wrap std::string are in place exactly to disambiguate between the various meanings we give to std::string (first name, last name, …), but as regards the date of birth, that's the only date in the API, so it needn't be disambiguated, hence std::chrono::year_month_day seemed just a good enough type to me; clearly, if you had more dates in the API, say birth date and death date, you could certainly wrap them in custom types similarly to what I've done for std::string.
• I'm not sure I've understood your concern about _maiden. In your example you have

  .addMaidenName(null)      //Forced to explicitly set null, a good thing
  

  so I assumed you do want to force the user to set maiden name, even if it is just null; in my solution, std::nullopt is playing the role of null, and its not necessary to make it special to the maiden case: an empty optional is the same thing whether we're dealing with optional<SomeType> or with optional<AnotherType>. If you really want to avoid the uninformative std::nullopt, you could either write std::optional<MaidenName>{} or, far better, define

  constexpr auto noMaidenName/* or whatever you like */ = std::nullopt;
  

  somewhere and use that. Actually, since no type deduction is going on, you can also just pass {} instead of std::nullopt. If, instead, what concerns you is the expression maiden.value_or("null"_maiden).str, that's because the constructors expect a std::string maiden name, and, unlike in Java (from your code I understand that you can assign null to a String), in C++ you can't assign a null-but-non-std::string value to a std::string, so the expression I used is to pull the std::string out of maiden if maiden.has_value() == true, and to pull it out of "null"_maiden otherwise, and in the latter case the string is trivially "null".