10

I am trying to use Builder Pattern for my class..

Below is my Builder class which I have built by following Joshua Bloch version as he showed in Effective Java, 2nd Edition. Our customer will mostly pass userId, clientId always but other fields are optional and they may or may not pass it. Here Preference is an ENUM class having four fields in it.

public final class InputKeys {

    private long userid;
    private long clientid;
    private long timeout = 500L;
    private Preference pref;
    private boolean debug;
    private Map<String, String> parameterMap;

    private InputKeys(Builder builder) {
        this.userid = builder.userId;
        this.clientid = builder.clientId;
        this.pref = builder.preference;
        this.parameterMap = builder.parameterMap;
        this.timeout = builder.timeout;
        this.debug = builder.debug;
    }

    public static class Builder {
        protected final long userId;
        protected final long clientId;
        protected long timeout;
        protected Preference preference;
        protected boolean debug;
        protected Map<String, String> parameterMap;

        public Builder(long userId, long clientId) {
            this.userId = userId;
            this.clientId = clientId;
        }

        public Builder parameterMap(Map<String, String> parameterMap) {
            this.parameterMap = parameterMap;
            return this;
        }

        public Builder preference(Preference preference) {
            this.preference = preference;
            return this;
        }

        public Builder debug(boolean debug) {
            this.debug = debug;
            return this;
        }

        public Builder timeout(long timeout) {
            this.timeout = timeout;
            return this;
        }

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

Below is my ENUM class -

public enum Preference {
    PRIMARY,
    SECONDARY
}    

I am making a call like this to construct the InputKeys parameter -

InputKeys keys = new InputKeys.Builder(12000L, 33L).build();    
System.out.println(keys);

The only problem here I am seeing is, if the customers are not passing any timeout value, I need to have default timeout value set as 500 always but if they are passing any timeout value then it should override my default timeout value. And this is not working for me as when I see my keys in the debug mode, it always has timeout value as 0.

Is there anything I am missing? And also I am trying to make this class immutable and thread safe.. Is this a thread safe and immutable version of Builder or I have missed something?

UPDATE:-

Specific scenarios when people will be using this is we have a factory which customers are going to use to make a call to our code. We have a simple interface which one of our class is implementing it and then we have a factory by which we will call a certain method of that implementation which accepts this keys parameter.

So they will be using the below code in there application to make a call and it might be possible they will be running there application in a multithreaded environment.

InputKeys keys = new InputKeys.Builder(12000L, 33L).build();    

IClient client = ClientFactory.getInstance();
client.getData(keys);

And then InputKeys Builder can be used multiple times to construct the keys, it's not one time. Whatever userId and clientId they will be getting they will construct the InputKeys again.

AKIWEB
  • 19,008
  • 67
  • 180
  • 294

2 Answers2

7

Just initialize the timeout to 500 in the Builder. The timeout method will overwrite this value if it's called.

protected long timeout = 500L;
rgettman
  • 176,041
  • 30
  • 275
  • 357
  • Thanks. And I should remove it from the `InputKeys` class.. Correct? And how about immutability and thread safe? Does it look fine? – AKIWEB Jan 10 '14 at 20:48
  • I see no reason to have the `500` in the `InputKeys` class now. For immutability, I would make the `InputKeys` variables `final`. It's not necessary, but it makes it clear the values shouldn't change. If you ever decide to implement getters in `InputKeys`, return a copy of `parameterMap` or `Collections.unmodifiableMap(parameterMap)` to prevent modification of the map object. As for thread safety, I don't know the scenario under which you would share a `Builder` among multiple threads. – rgettman Jan 10 '14 at 21:05
  • Thanks. Somehow Collections.unmodifiableMap(parameterMap) was giving me NPE when I was trying to instantiate the InputKeys class so I removed it.. Also I have updated my question as well. – AKIWEB Jan 10 '14 at 21:10
  • I would add a check in the `build` method to ensure that `parameterMap` isn't `null`. If it is, throw an exception (IllegalStateException?). If the caller is unwilling or unable to supply a map via the `parameterMap` method, then you can initialize the instance variable `parameterMap` in the `Builder` yourself, just as we did for `timeout`. – rgettman Jan 10 '14 at 21:14
4

You can initialize the timeout value in the Builder constructor (or at variable declaration), e.g.

public Builder(long userId, long clientId) {
    this.userId = userId;
    this.clientId = clientId;
    this.timeout = 500L;
}

This way, if you don't call the timeout() method, it will have the default value, otherwise it will be set to the value passed via timeout().

Regarding thread safety, if the Builder constructor is only used for creating a new InputKeys object (as is in the scenario described in the question), without any other area of the code having access to that InputKeys object, there is no need to apply any extra protection.

PNS
  • 19,295
  • 32
  • 96
  • 143
  • Thanks you for your suggestions. I just updated the question with more details. – AKIWEB Jan 10 '14 at 21:09
  • Builder constructor can be used multiple times as well. It's not only one call. `userId` will keep on changing always for a every call and if they decide to use other parameters as well, then it will also go with every call.. – AKIWEB Jan 10 '14 at 21:19
  • Using the Builder constructor multiple times is fine, as long as you don't share the InputKeys object created by it among different threads. Every time you use the constructor, a new InputKeys object will be produced. – PNS Jan 14 '14 at 17:34
  • this is flawless PNS! – Gaurav Mar 15 '19 at 19:52