2

Let's say I have a class Dot with a builder:

public class Dot {
    private final Double x;
    private final Double y;
    private final Color color;

    private Dot(Double x, Double y, Color color) {
        this.x = x;
        this.y = y;
        this.color = color;
    }

    public static Builder createBuilder() {
        return new Builder();
    }

    public Double getX() { return x; }

    public Double getY() { return y; }

    public Color getColor() { return color; }

    public Builder getBuilder() {
        return new Builder().setX(x).setY(y).setColor(color);
    }

    public static class Builder {
        private Double x;
        private Double y;
        private Color color;

        public Builder setX(Double x) {
            this.x = x;
            return this;
        }

        public Builder setY(Double y) {
            this.y = y;
            return this;
        }

        public Builder setColor(Color color) {
            this.color = color;
            return this;
        }

        public Dot build() {
            return new Dot(x, y, color);
        }
    }
}

Now I see two possible ways one could instantiate the builder. Through its constructor:

new Dot.Builder().setX(...

or through a factory method:

Dot.createBuilder().setX(...

(In the latter case I would make the Builder's constructor private.)

Factory methods (or factories) are (to my knowledge) useful when the actual type of the returned value might change later on. This is something that is probably not going to occur with a builder.

My best argument for using a factory method would be: Say that the builder uses a final field String id that has to be set in its constructor, but certain restrictions apply to what constitutes an ID. Throwing an exception on an illegal id string should not be done in a constructor. Therefore a factory method would be better.

However I feel like I missed some more important arguments. Does a best practice for this case exist? What other arguments exist for/against using a factory method here?

  • 3
    Despite what your link says, throwing exceptions in constructors is very good style. It's the easiest way to write robust code as every object is responsible for its own correctness. The link merely states that an object can still exist after constructor validation failed, which makes ctor validation unsuitable for security checks if you run untrusted code in the same JVM where the attacker can extend your class. Under all other circumstances, exceptions in constructors are great. – amon Dec 13 '17 at 7:20
  • What's the use case for introducing a builder or a factory instead of directly using the Dot(x,y,color) constructor? Create some new Dot? Create a copy? Create a moved or differently-colored copy? Or is "Dot" just a simplified example? – Ralf Kleberhoff Dec 13 '17 at 10:29
2

Builder is a class internal to Dot. For me this tells me that Builder is strongly coupled with Dot and that Builder is a utility of Dot for some objective, in this case object construction.

This is objectively one step away from Builder being an internal class hidden away from the caller entirely. The only link you, the caller, should have to Builder should be through Dot as a general rule. And although there's nothing stopping you from making Builder's constructor public and accessing it directly, I confirmly believe it is an antipattern.

Therefore, you should have a static factory method in Dot that returns an instance of Builder. You mention that it isn't likely that you'd ever need to extend Builder, and that's all fine and good, but when you're given a more flexible option with no drawbacks while you're developing, you grab hold of it like your life depended on it. Should you ever need to extend Builder, you could make Builder<T> an interface and create a customized DotBuilder<Dot> which implements it which when called creates an instance of Dot.

My best argument for using a factory method would be: Say that the builder uses a final field String id that has to be set in its constructor, but certain restrictions apply to what constitutes an ID. Throwing an exception on an illegal id string should not be done in a constructor. Therefore a factory method would be better.

I'm usually against constructors performing work that might likely throw exceptions. It's one thing to throw an exception in a constructor for an invalid parameter or for a very unlikely situation such as Xml implementation not existing, and quite another to attempt to establish a database connection which can and will fail quite easily. If you did need to do work in your Builder constructor, you could do so lazily putting it off until the actual creation of the object as to not make your constructor explode. Though if I've got Builder class pegged correctly, it isn't the type of class that should ever explode upon creation. If that isn't your case, give serious thought to making it work in the way that one might expect it to work (aka possible exceptions when calling build() method only).

Also if I may add, having a builder class Builder with many of the same methods as Dot is not probably a good idea. Your situation may be different and this may be the simplified version, so if this doesn't apply to you, ignore this advice. Though it is my opinion that Builder is largely unnecessary here. Dot can return this after each setter and perform precisely the same functionality. Give it some thought.

  • 2
    In OP's design, the Dot class is immutable. Adding setters to make it mutable would drastically change the design. Under such circumstances a builder does have value. Regarding validation: performing validation before a (private) ctor is called can be sensible. But wouldn't lazy validation after the ctor make the system terribly unpredictable? I've had to use that strategy once on a larger scale and it was a total pain due to convoluted data flows. Btw I don't think asserting object invariants violates the point of the “no behaviour in constructors” guideline. – amon Dec 13 '17 at 8:13
  • 1
    @amom When I say "throw an exception in a constructor for an invalid parameter", I mean to say parameter validation in general. I don't think anything is wrong with making sure the parameters are proper. Though there should be nothing of the sort for Builder since it doesn't have parameters in this case. Also, it's true, Dot is immutable, and that's certainly advantageous. It depends on his design, but I would argue that it is a little excessive for a simple bean class with no business logic. – Neil Dec 13 '17 at 9:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.