11

Edit: I'd like to point out that this question describes a theoretical problem, and I am aware that I can use constructor arguments for mandatory parameters, or throw a runtime exception if the API is used incorrectly. However, I am looking for a solution that does not require constructor arguments or runtime checking.

Imagine you have a Car interface like this:

public interface Car {
    public Engine getEngine(); // required
    public Transmission getTransmission(); // required
    public Stereo getStereo(); // optional
}

As the comments would suggest, a Car must have an Engine and Transmission but a Stereo is optional. That means a Builder that can build() a Car instance should only ever have a build() method if an Engine and Transmission have both already been given to the builder instance. That way the type checker will refuse to compile any code that attempts to create a Car instance without an Engine or Transmission.

This calls for a Step Builder. Typically you'd implement something like this:

public interface Car {
    public Engine getEngine(); // required
    public Transmission getTransmission(); // required
    public Stereo getStereo(); // optional

    public class Builder {
        public BuilderWithEngine engine(Engine engine) {
            return new BuilderWithEngine(engine);
        }
    }

    public class BuilderWithEngine {
        private Engine engine;
        private BuilderWithEngine(Engine engine) {
            this.engine = engine;
        }
        public BuilderWithEngine engine(Engine engine) {
            this.engine = engine;
            return this;
        }
        public CompleteBuilder transmission(Transmission transmission) {
            return new CompleteBuilder(engine, transmission);
        }
    }

    public class CompleteBuilder {
        private Engine engine;
        private Transmission transmission;
        private Stereo stereo = null;
        private CompleteBuilder(Engine engine, Transmission transmission) {
            this.engine = engine;
            this.transmission = transmission;
        }
        public CompleteBuilder engine(Engine engine) {
            this.engine = engine;
            return this;
        }
        public CompleteBuilder transmission(Transmission transmission) {
            this.transmission = transmission;
            return this;
        }
        public CompleteBuilder stereo(Stereo stereo) {
            this.stereo = stereo;
            return this;
        }
        public Car build() {
            return new Car() {
                @Override
                public Engine getEngine() {
                    return engine;
                }
                @Override
                public Transmission getTransmission() {
                    return transmission;
                }
                @Override
                public Stereo getStereo() {
                    return stereo;
                }
            };
        }
    }
}

There's a chain of different builder classes (Builder, BuilderWithEngine, CompleteBuilder), that add one required setter method after another, with the last class containing all optional setter methods as well.
This means that users of this step builder are confined to the order in which the author made mandatory setters available. Here is an example of possible uses (note that they are all strictly ordered: engine(e) first, followed by transmission(t), and finally the optional stereo(s)).

new Builder().engine(e).transmission(t).build();
new Builder().engine(e).transmission(t).stereo(s).build();
new Builder().engine(e).engine(e).transmission(t).stereo(s).build();
new Builder().engine(e).transmission(t).engine(e).stereo(s).build();
new Builder().engine(e).transmission(t).stereo(s).engine(e).build();
new Builder().engine(e).transmission(t).transmission(t).stereo(s).build();
new Builder().engine(e).transmission(t).stereo(s).transmission(t).build();
new Builder().engine(e).transmission(t).stereo(s).stereo(s).build();

However, there are plenty of scenarios in which this is not ideal for the builder's user, especially if the builder has not just setters, but also adders, or if the user cannot control the order in which certain properties for the builder will become available.

The only solution I could think of for that is very convoluted: For every combination of mandatory properties having been set or having not yet been set, I created a dedicated builder class that knows which potential other mandatory setters need to be called before arriving at a state where the build() method should available, and each one of those setters returns a more complete type of builder which is one step closer to containing a build() method.
I added the code below, but you might say that I'm using the type system to create a FSM that lets you create a Builder, which can be turned into either a BuilderWithEngine or BuilderWithTransmission, which both can then be turned into a CompleteBuilder, which implements the build() method. Optional setters can be invoked on any of these builder instances. enter image description here

public interface Car {
    public Engine getEngine(); // required
    public Transmission getTransmission(); // required
    public Stereo getStereo(); // optional

    public class Builder extends OptionalBuilder {
        public BuilderWithEngine engine(Engine engine) {
            return new BuilderWithEngine(engine, stereo);
        }
        public BuilderWithTransmission transmission(Transmission transmission) {
            return new BuilderWithTransmission(transmission, stereo);
        }
        @Override
        public Builder stereo(Stereo stereo) {
            super.stereo(stereo);
            return this;
        }
    }

    public class OptionalBuilder {
        protected Stereo stereo = null;
        private OptionalBuilder() {}
        public OptionalBuilder stereo(Stereo stereo) {
            this.stereo = stereo;
            return this;
        }
    }

    public class BuilderWithEngine extends OptionalBuilder {
        private Engine engine;
        private BuilderWithEngine(Engine engine, Stereo stereo) {
            this.engine = engine;
            this.stereo = stereo;
        }
        public CompleteBuilder transmission(Transmission transmission) {
            return new CompleteBuilder(engine, transmission, stereo);
        }
        public BuilderWithEngine engine(Engine engine) {
            this.engine = engine;
            return this;
        }
        @Override
        public BuilderWithEngine stereo(Stereo stereo) {
            super.stereo(stereo);
            return this;
        }
    }

    public class BuilderWithTransmission extends OptionalBuilder {
        private Transmission transmission;
        private BuilderWithTransmission(Transmission transmission, Stereo stereo) {
            this.transmission = transmission;
            this.stereo = stereo;
        }
        public CompleteBuilder engine(Engine engine) {
            return new CompleteBuilder(engine, transmission, stereo);
        }
        public BuilderWithTransmission transmission(Transmission transmission) {
            this.transmission = transmission;
            return this;
        }
        @Override
        public BuilderWithTransmission stereo(Stereo stereo) {
            super.stereo(stereo);
            return this;
        }
    }

    public class CompleteBuilder extends OptionalBuilder {
        private Engine engine;
        private Transmission transmission;
        private CompleteBuilder(Engine engine, Transmission transmission, Stereo stereo) {
            this.engine = engine;
            this.transmission = transmission;
            this.stereo = stereo;
        }
        public CompleteBuilder engine(Engine engine) {
            this.engine = engine;
            return this;
        }
        public CompleteBuilder transmission(Transmission transmission) {
            this.transmission = transmission;
            return this;
        }
        @Override
        public CompleteBuilder stereo(Stereo stereo) {
            super.stereo(stereo);
            return this;
        }
        public Car build() {
            return new Car() {
                @Override
                public Engine getEngine() {
                    return engine;
                }
                @Override
                public Transmission getTransmission() {
                    return transmission;
                }
                @Override
                public Stereo getStereo() {
                    return stereo;
                }
            };
        }
    }
}

As you can tell, this doesn't scale well, as the number of different builder classes required would be O(2^n) where n is the number of mandatory setters.

Hence my question: Can this be done more elegantly?

(I'm looking for an answer that works with Java, although Scala would be acceptable too)

derabbink
  • 210
  • 2
  • 7
  • 1
    What prevents you from just using an IoC container to stand up all of these dependencies? Also, I'm not clear why, if order doesn't matter as you have asserted, you couldn't just use ordinary setter methods that return `this`? – Robert Harvey Jul 07 '16 at 23:17
  • What does it mean to invoke `.engine(e)` twice for one builder? – Erik Eidt Jul 07 '16 at 23:32
  • 3
    If you want to verify it statically without manually writing a class for every combination, you probably have to use neckbeard-level stuff like macros or template metaprogramming. Java isn't expressive enough for that, to my knowledge, and the effort likely wouldn't be worth it over dynamically verified solutions in other languages. – Karl Bielefeldt Jul 08 '16 at 01:14
  • 1
    Robert: The goal is to have the type checker enforce the fact that both an engine and a transmission are mandatory; this way you can't even call `build()` if you haven't called `engine(e)` and `transmission(t)` before. – derabbink Jul 08 '16 at 07:30
  • Erik: You might want to start with a default `Engine` implementation, and later overwrite it with a more specific implementation. But most likely this would make more sense if `engine(e)` wasn't a setter, but an adder: `addEngine(e)`. This would be useful for a `Car` builder that can produce hybrid cars with more than one engine/motor. Since this is a contrived example, I didn't go into details on why you might want to do this - for brevity. – derabbink Jul 08 '16 at 07:33
  • Karl: I think you might be right. And I think - even if I were to use such "neckbeard-level stuff" - the overhead caused by the ballooning number builder classes would diminish the added convenience this would offer for the builder's users. – derabbink Jul 08 '16 at 07:36
  • I'm curious, could you not just hide this construction logic from clients and do it in a factory you control and can make sure all code paths that build a car buillds a valid one? also, probably unrelated, but why would a client using the `Car` interface want to be able to open up the hood and take a look at the engine and fiddle about with the transmission? this LOOKS like a violation of "tell, don't ask". just thought it's worth thinking about. – sara Jul 08 '16 at 07:44
  • kai: Can you elaborate on what you'd make such a factory look like? Also, the `getEngine()`, `getTransmission()` and `getStereo()` on the `Car` interface are not the core of the quesion; Imagine they don't exist, and that the `Car` has a `default void drive()` method that uses the `engine`, `transmission`, and `stereo` instances instead. – derabbink Jul 08 '16 at 09:48
  • I think you have an option to generate all those Builder classes by creating some kind of annotation processor for Java. – Juozas Kontvainis Jul 13 '16 at 19:20
  • @KarlBielefeldt This is not the case. See the link in my answer below for a solution in Java. http://softwareengineering.stackexchange.com/a/343612/186008 – breandan Mar 07 '17 at 05:32
  • @breandan, you've misunderstood the question. He has started from the builder pattern and found it insufficient for his needs. He wants the `.build()` to be statically unavailable until all the components are in place, no matter what order you specify the components, which is a much more difficult problem than the builder pattern addresses. – Karl Bielefeldt Mar 07 '17 at 14:58
  • Didn't misunderstand. That is exactly what the Java and Scala code in the examples provide, with a little adaptation. I've written an example in Kotlin to illustrate: https://gist.github.com/breandan/d0d7c21bb7f78ef54c21ce6a6ac49b68 – breandan Mar 07 '17 at 15:37
  • @KarlBielefeldt Some moderator banned me from answering this topic, so I'm adding a link to my answer here: https://gist.github.com/breandan/7a76161541546278fbb7961948ac5bb8 – breandan Mar 08 '17 at 03:51
  • I stand corrected. You'll be able to answer this question if you go answer another that gets upvoted, then come back. – Karl Bielefeldt Mar 08 '17 at 05:14

4 Answers4

4

You seem to have two different requirements, based on the method calls you provided.

  1. Only one (required) engine, only one (required) transmission, and only one (optional) stereo.
  2. One or more (required) engines, one or more (required) transmissions, and one or more (optional) stereos.

I think the first issue here is that you don't know what you want the class to do. Part of that is that it's not known what you want the built object to look like.

A car can only have one engine and one transmission. Even hybrid cars only have one engine (perhaps a GasAndElectricEngine)

I'll address both implementations:

public class CarBuilder {

    public CarBuilder(Engine engine, Transmission transmission) {
        // ...
    }

    public CarBuilder setStereo(Stereo stereo) {
        // ...
        return this;
    }
}

and

public class CarBuilder {

    public CarBuilder(List<Engine> engines, List<Transmission> transmission) {
        // ...
    }

    public CarBuilder addStereo(Stereo stereo) {
        // ...
        return this;
    }
}

If an engine and transmission are required, then they should be in the constructor.

If you don't know what engine or transmission is required, then don't set one yet; it's a sign that you're creating the builder too far up the stack.

Zymus
  • 2,403
  • 3
  • 19
  • 35
2

Why not using the null object pattern ? Get rid of this builder, the most elegant code you can write is the one you actually don't have to write.

public final class CarImpl implements Car {
    private final Engine engine;
    private final Transmission transmission;
    private final Stereo stereo;

    public CarImpl(Engine engine, Transmission transmission) {
        this(engine, transmission, new DefaultStereo());
    }

    public CarImpl(Engine engine, Transmission transmission, Stereo stereo) {
        this.engine = engine;
        this.transmission = transmission;
        this.stereo = stereo;
    }

    //...

}
Spotted
  • 1,680
  • 10
  • 18
  • This is what I thought straight away. although I wouldn't have the three parameter constructor, just the two parameter constructor that has the mandatory elements and then a setter for the stereo as it is optional. – Encaitar Jul 08 '16 at 14:17
  • 1
    In a simple (contrived) example like `Car`, this would make sense as the number of c'tor arguments is very small. However, as soon as you're dealing with anything moderately complex (>=4 mandatory arguments), the whole thing gets more difficult to handle/less readable ("Did the engine or the transmission come first?"). That's why you would use a builder: The API forces you to be more explicit about what you're constructing. – derabbink Jul 08 '16 at 14:31
  • 1
    @derabbink Why not breaking your class in smaller ones in this case ? Using a builder will just hide the fact that the class is doing too much and has become unmaintainable. – Spotted Jul 08 '16 at 14:35
  • 1
    Kudos for ending the pattern madness. – Robert Harvey Jul 08 '16 at 15:06
  • @Spotted some classes just contain a lot of data. For example, if you want to produce an access log class that holds all the related info about the HTTP request and output the data into CSV or JSON format. There will be a lot of data and if you want to enforce some fields to be present during the compile time, you'll need a builder pattern with a very long arg list constructor, which doesn't look good. – ssgao Feb 26 '19 at 07:18
1

Firstly, unless you have a lot more time than any shop I've worked in, it is probably not worth allowing any order of operations or just living with the fact that you can specify more than one radio. Note that you are talking about code, not user input, so you can have assertions that will fail during your unit testing rather than a second before at compile time.

However, if your constraint is, as given in the comments, that you have to have an engine and a transmission, then enforce this by putting all the obligatory properties is the builder's constructor.

new Builder(e, t).build();                      // ok
new Builder(e, t).stereo(s).build();            // ok
new Builder(e, t).stereo(s).stereo(s).build();  // exception on second call to stereo as stereo is already set 

If it is only the stereo which is optional, then doing the final step using sub-classes of builders is possible, but beyond that the gain from getting the error at compile time rather than in testing is probably not worth the effort.

Pete Kirkham
  • 1,798
  • 14
  • 15
0

the number of different builder classes required would be O(2^n) where n is the number of mandatory setters.

You have already guessed the correct direction for this question.

If you want to get compile-time checking, you will need (2^n) types. If you want to get run-time checking, you will need a variable that can store (2^n) states; an n-bit integer will do.


Because C++ supports non-type template parameter (e.g. integer values), it is possible for a C++ class template to be instantiated into O(2^n) different types, using a scheme similar to this.

However, in languages that don't support non-type template parameters, you cannot rely on the type system to instantiate O(2^n) different types.


The next opportunity is with Java annotations (and C# attributes). These additional metadata can be used to trigger user-defined behavior at compile-time, when annotation processors are used. However, it would be too much work for you to implement these. If you are using frameworks that provide this functionality for you, use it. Otherwise, check the next opportunity.


Finally, notice that storing O(2^n) different states as a variable at run-time (literally, as an integer that has at least n bits wide) is very easy. This is why the most upvoted answers all recommend you to perform this check at run-time, because the effort needed to implement compile-time checking is too much, when compared to the potential gain.

rwong
  • 16,695
  • 3
  • 33
  • 81