How to initialize a variable in the constructor only once?

Question

I have a builder pattern in which I am taking few parameters from the customer and basis on that I am building my builder class and then that builder class is passed to our underlying library and then my library will use it.

public final class KeyHolder {
  private final String clientId;
  private final String deviceId;
  private final int processId;
  private final Cache<String, List<Response>> userCache;
  private static final long MAXIMUM_CACHE_SIZE = 5000000;
  private static final long EXPIRE_AFTER_WRITE = 120; // this is in seconds

  private KeyHolder(Builder builder) {
    this.clientId = builder.clientId;
    this.deviceId = builder.deviceId;
    this.processId = builder.processId;
    this.maximumCacheSize = builder.maximumCacheSize;
    this.expireAfterWrite = builder.expireAfterWrite;

    // how to execute this line only once
    this.userCache =
        CacheBuilder
            .newBuilder()
            .maximumSize(maximumCacheSize)
            .expireAfterWrite(expireAfterWrite, TimeUnit.SECONDS)
            .removalListener(
                RemovalListeners.asynchronous(new CustomListener(),
                    Executors.newSingleThreadScheduledExecutor())).build();

  }

  public static class Builder {
    protected final int processId;
    protected String clientId = null;
    protected String deviceId = null;
    protected long maximumCacheSize = MAXIMUM_CACHE_SIZE;
    protected long expireAfterWrite = EXPIRE_AFTER_WRITE;


    public Builder(int processId) {
      this.processId = processId;
    }

    public Builder setClientId(String clientId) {
      this.clientId = clientId;
      return this;
    }

    public Builder setDeviceId(String deviceId) {
      this.deviceId = deviceId;
      return this;
    }

    public Builder setMaximumCacheSize(long size) {
      this.maximumCacheSize = size;
      return this;
    }

    public Builder setExpiryTimeAfterWrite(long duration) {
      this.expireAfterWrite = duration;
      return this;
    }

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

 // getters here
}

For each and every call to our library they create a new KeyHolder builder class everytime and pass it to our library. processId, clientId, deviceId will change with every call but maximumCacheSize and expireAfterWrite will stay same as it with every call. As you can see above, I am using guava cache here and since they are creating KeyHolder builder class everytime how can I make sure that the below line is executed only once in my constructor?

    this.userCache =
        CacheBuilder
            .newBuilder()
            .maximumSize(maximumCacheSize)
            .expireAfterWrite(expireAfterWrite, TimeUnit.SECONDS)
            .removalListener(
                RemovalListeners.asynchronous(new CustomListener(),
                    Executors.newSingleThreadScheduledExecutor())).build();

Since with the current code right now, it will get executed with every call and I will get a new guava cache every time in my library so whatever entry was cached earlier within my library by using this guava cache will get lost.

How to initialize a particular variable only once and after that it should ignore the value whatever is being passed to it?

Update:

public class DataClient implements Client {
    private final ExecutorService executor = Executors.newFixedThreadPool(10);

    // for synchronous call
    @Override
    public List<Response> executeSync(KeyHolder key) {
        Cache<String, List<Response>> userCache = key.getUserCache();
        List<Response> response = userCache.getIfPresent(key.getUUID());
        if (CollectionUtils.isNotEmpty(response)) {
          return response;
        }
        // if not in cache, then normally call the flow and populate the cache
        List<Response> dataResponse = null;
        Future<List<Response>> future = null;
        try {
            future = executeAsync(key);
            dataResponse = future.get(key.getTimeout(), TimeUnit.MILLISECONDS);
            userCache.put(key.getUUID(), dataResponse);
        } catch (TimeoutException ex) {
            // log error and return DataResponse
        } catch (Exception ex) {
            // log error and return DataResponse
        }

        return dataResponse;
    }
}

Answer 1

If you only want to set the cache once, why does every KeuHolder object attempt to build it? In fact, even KeyHolder#Builder exposes methods to help the construction of the cache, which would only be useful once.

This is highly questionable. What if the first KeyHolder doesn't specify the cache details? I mean, it's not forced to (you aren't using the builder pattern correctly, more on that at the end).

The first step to solving this issue would be to ensure the cache is set before you start creating KeyHolder object's. You can do this by creating a static factory and making userCache static:

class KeyHolder {
    private static Map<String, List<Response>> userCache;

    public static KeyHolder.Builder newBuilder(int id) {
        if(userCache == null) {
            userCache = ...;
        }

        return new Builder(id);
    }
}

But as you've probably read from my comments, this is simply a band-aid for the issue. This checks the userCache every time we want to create a new KeyHolder, which shouldn't need to happen.

Instead, you should decouple the cache from KeyHolder all together. Why does it need to know about caching anyways?

Your cache belongs in DataClient:

class DataClient {
    private Map<String, List<Response>> userCache;

    public List<Response> executeSync(KeyHolder key) {
        List<Response> response = userCache.getIfPresent(key.getUUID());
        //...
    }
}

You could accept the settings via DataClient constructor, or pass the cache into DataClient with the settings already specified.

---

As for your use of the builder pattern, keep in mind why we use it: Java lacks optional parameters.

That's why builders are common: they allow us to specify optional data via methods.

You are specifying critical info, such as cache settings, as optional parameters (builder methods). You should only use builder methods if you do not require the information, and cache information is definitely something that should be required. I'd question how optional deviceId and clientId are aswell, seeing how the only required data is productId.

Answer 2

first, make it static

private static Cache<String, List<Response>> userCache;

then, initialize it only if it hasn't been initialized

private KeyHolder(Builder builder) {
    ...

    if (userCache == null) {
        userCache = CacheBuilder
            .newBuilder()
            .maximumSize(maximumCacheSize)
            .expireAfterWrite(expireAfterWrite, TimeUnit.SECONDS)
            .removalListener(
                RemovalListeners.asynchronous(new CustomListener(), Executors.newSingleThreadScheduledExecutor())
            ).build();
    }
}

if you don't really need the cache to be customization at run-time (by using passed in parameters, I'd suggest going with something like

// initialize the cache while defining it
// replace maximumCacheSize and expireAfterWrite with constants
private static final Cache... = CacheBuilder.newBuilder()...;

and remove it from the constructor.

Answer 3

You can make the variable static and only initialize it on the first call, if it is null. Something like this:

public final class KeyHolder {
  private final String clientId;
  private final String deviceId;
  private final int processId;

  //this var is now static, so it is shared across all instances
  private static Cache<String, List<Response>> userCache = null;  

  private static final long MAXIMUM_CACHE_SIZE = 5000000;
  private static final long EXPIRE_AFTER_WRITE = 120; // this is in seconds

  private KeyHolder(Builder builder) {
    this.clientId = builder.clientId;
    this.deviceId = builder.deviceId;
    this.processId = builder.processId;
    this.maximumCacheSize = builder.maximumCacheSize;
    this.expireAfterWrite = builder.expireAfterWrite;

    //this will be executed only the first time, when the var is null
    if (userCache == null) {
      userCache =
        CacheBuilder
            .newBuilder()
            .maximumSize(maximumCacheSize)
            .expireAfterWrite(expireAfterWrite, TimeUnit.SECONDS)
            .removalListener(
                RemovalListeners.asynchronous(new CustomListener(),
                    Executors.newSingleThreadScheduledExecutor())).build();

  }

  //rest of your class below