Updated: Questions on implementing Builder design pattern

Question

I am in the process of using Java Builder design pattern to write a connector class that has optional member variables and need some suggestions. My question from here Standard practices for employing Java Builder design pattern has been updated and the code now presented is not psuedocode but is specific.

public class Connector {

    // Instance variables
    String appUrl = null;
    String oneTimePwd = null;
    String accessKey = null;

    // Private constructor
    private Connector(Builder builder) {
        this.appUrl = builder.appUrl;
        this.accessKey = builder.accessKey;
        this.oneTimePwd = builder.oneTimePwd;
    }

    public static class Builder {

        // Instance variables
        String appUrl = null;
        String accessKey = null;
        String oneTimePwd = null;

        public Builder() {
            // Empty constructor that accepts no arguments.
        }

        // Setter method for mandatory field/s
        public Builder setMandatoryParameters(String appUrl) {
            this.appUrl = appUrl;
            return this;
        }

        // Setter method1 for optional field/s
        public Builder setAccessKey(String accessKey) {
            this.accessKey = accessKey;
            return this;
        }

        // Setter method2 for optional field/s
        public Builder setOneTimePwd(String oneTimePwd) {
            this.oneTimePwd = oneTimePwd;
            return this;
        }

        public Connector Build() {
            if (accessKey != null) {
                return new Connector(this);
            } else if (oneTimePwd != null) {
                return new Connector(this);
            } else {
                // log.error("Either Access key or One time password must be specified.")
            }
            return null;
        }
    }

    // Other methods of the Connector class
    // ...
}

Question 1: Is it sensible to make use of a method "setMandatoryParameters()" to obtain values for mandatory parameters? This will allow creation of an instance of the Builder without supplying the mandatory parameters which can be a drawback. Is it much better to ask the user to supply the values for mandatory parameters with Builder's constructor?

Question 2: Either access key or one time password is needed to establish a successful connection. Thus, field validations such as null checks have been employed in the "Build()" method. Should such validations be performed in the Connector class's constructor instead?

Answer 1

Design improvements

Even with the Builder pattern, the target class shall only provide constructors that allow creation of valid objects. You still need two different constructors: one for constructing the connector with an access key and one for OTP.

The Builder's responsibility is to provide setters and to check at build time that both access key and OTP are not set at the same time and then call the correct constructor. It should never pass itself to the target class. Just the parameters.

SOLID

Your Connector does a lot of things. Instead of having one concrete Connector class for both access key and OTP, you should implement the Connector as an interface or abstract class and have the builder create an AccessKeyConnector or OtpConnector depending on the parameters that were provided to it.

Style

The setMandatoryParameters is a bad name for a method. It should communicate what the parameter does, not if it is mandatory or not. Use setAppUri instead.

Answer 2

Question 1: If the parameters are mandatory anyway you might as well enforce this through your API. This reads just fine:

Connector con = new Connector.Builder(mandatoryURL)
                    .setAccessKey(key)
                    .build();

Question 2: Either place, since you contained the checks entirely in the Connector class (because of the private constructor) you have full control anyway.

Since either one of them is mandatory and you I'm assuming you won't use both of them, you might also provide 2 separate "build" methods that take one of those parameters as input.

public static class Builder {
    String appUrl = null;

    public Builder(String appUrl) {
        this.appUrl = appUrl;
    }

    public Connector withAccessKey(String accessKey) {
        Connector result = new Connector(appUrl);
        connector.accessKey = accessKey;
        return connector;
    }

    public Connector withOneTimePwd(String oneTimePwd) {
        Connector result = new Connector(appUrl);
        connector.oneTimePwd = oneTimePwd;
        return connector;
    }
}

Quick note that I changed the Connector constructor to also take the URL parameter instead of the builder. Your way works just fine in this case, I just prefer to have the builder be responsible for setting the correct fields instead of making the Connector "know" about the Builder class.

---

Alternative solution to using a Builder is to use factory methods. Your Connector class could then look something like this:

public class Connector {

    String appUrl = null;
    String oneTimePwd = null;
    String accessKey = null;

    private Connector(String appUrl) {
        this.appUrl = appUrl;
    }

    public static Connector createWithAccessKey(String appUrl, String accessKey) {
        Connector result = new Connector(appUrl);
        connector.accessKey = accessKey;
        return connector;
    }

    public static Connector createWithOneTimePwd(String appUrl, String oneTimePwd) {
        Connector result = new Connector(appUrl);
        connector.oneTimePwd = oneTimePwd;
        return connector;
    }

    //... rest of the class
}

You can now use it as follows:

Connector con = Connector.createWithAccessKey(someUrl, myKey);

This has about the same effect as your Builder idea but with a lot less boilerplate code to reach it.

Answer 3

It is fine to create Builder without passing mandatory parameters in it's constructor. There can be cases, where you don't know all parameters at creation of builder, but you know them later before calling .build().

What I am really missing here is validation, that object will be in correct state at the beginning of build method. That method should imho always either create valid object or throw exception. You can also choose what kind of instance type to return afterwards as suggested by @JAD. But you seriously don't wanna deal with invalid or null objects returned by your builder.

Answer 4

One aspect that you are missing here is that most likely, the functionality of Connector will depend on whether it was passed a oneTimePwd or an accessKey. It could even have been passed both. What would happen then?

Imus' answer provides some good insights for the builder side of the code, but you can also leverage the other side. By turning Connector into an abstract class that defines its interface, you can create two separated implementations of that abstract class, depending on the type of key that was passed. One of the strengths of the builder pattern is that you can easily return either of those two types, without having to bother the user of the class with the choice between them:

    public Connector Build() {
        if (accessKey != null) {
            return new AccessKeyConnector(this);
        } else if (oneTimePwd != null) {
            return new OneTimePasswordConnector(this);
        } else {
            // log.error("Either Access key or One time password must be specified.")
        }
        return null;
    }