[FIXED] How defer Map value object creation when key is fetched

Issue

We had a factory method like below, which worked fine, but when the case statements increased, We had issues with SONAR.

class MessageFactory{
public Message create(int uuid) {
switch (uuid) {
  case FIRST_MESSAGE_ID:
    return new FirstMessage();
    .
  case SECOND_MESSAGE_ID:
    return new SecondMessage();
    .
  default:

      return null;
}}}

class Main{
MessageFactory factory = new MessageFactory();
Message message1=factory.get(FIRST_MESSAGE_ID); // needs a new instance everytime
.
.
MessageFactory factory = new MessageFactory();
Message message2=factory.get(FIRST_MESSAGE_ID); // needs a new instance everytime
}

To avoid the SONAR complaints we moved the object creation to a map.

public Message create(int uuid) {
Map<Integer, Message> map = new HashMap<>();
map.put(FIRST_MESSAGE_ID, new FirstMessage());
.
.
.
map.put(SECOND_MESSAGE_ID, new SecondMessage());
}
if(map.containsKey(uuid)){
return map.get(uuid);
}
return null;
}

The only drawback of this is that every time when I try creating objects using create(int uuid), the whole "map" is initialized every time with all my 100+ objects. Is there any way to defer creating objects only when the key is fetched? Or any other optimized way to solve this issue.

NOTE: I need a new instance of objects everytime I cannot use a static map, as it will return the same object instance every time, instead of creating a new object for that key when asked for.

Can we use lambda functions here to fetch new object everytime only when the key is fetched?

Solution

The only drawback of this is that every time when I try creating objects using create(int uuid), the whole "map" is initialized every time with all my 100+ objects

Firstly, declare the map as a static field (it seems like its contents is unrelated to a particular instance, if so it should be static). It would be generated only once when the class gets loaded into memory.

Secondly, extract the logic responsible for initializing the map into a separate method.

NOTE: I need a new instance of objects everytime

To meat this requirement, the Map should store not actual objects, but Suppliers for every kind of Message.

That's how it might be implemented:

public static final Map<Integer, Supplier<Message>> MESSAGE_BY_UUID = init();

public static Map<Integer, Supplier<Message>> init() {
    Map<Integer, Supplier<Message>> map = new HashMap<>();
    // your initialization logic
    map.put(FIRST_MESSAGE_ID, FirstMessage::new);
    map.put(SECOND_MESSAGE_ID, SecondMessage::new);
    ...
    return map;
}

public Message create(int uuid) {
    return MESSAGE_BY_UUID.get(uuid).get();
}

A simple Demo

Note that your if-else statement with map.containsKey check is redundant. Method get() would return null in case if provided key is not present in the map.

Spiking of other alternatives, the map can be initialized inline using method Map.ofEntries() which expects varargs of Map.Entry objects.

The downside of this approach is that it would be basically a switch in disguise, meaning that it would iterate over its keys in linear fashion. That's how it's implemented internally - it isn't hash-based, but maintains an array of alternating keys and values. With every invocation of the get() an immutable map returned by Map.ofEntries() would need to check up to 100+ UUID, in contrast with HashMap which would need to check only a couple (I said couple because collisions might occur).

Answered By - Alexander Ivanchenko
Answer Checked By - David Marino (JavaFixing Volunteer)