Flyweight pattern solution: CellContextFactory hash collisions

Design Patterns
1

There is a mild bug here since the Objects.hash function implemented here does nothing to prevent hash collisions. That is to say, there is no guarantee that two distinct CellContexts would not have the same hash so you could return the wrong one from the map.

Some options I can think of for fixing this are:

  1. Use a Map<Integer, List<CellContext>> and resolve the collisions manually by iterating through the list (means we should probably override hashCode and equals on the CellContext class).
  2. Instead of hashing, use nested maps: Map<String, Map<Integer, Map<Boolean, CellContext>>>. Still requires considerable manual work in there to deal with the missing partial cases (creating empty maps when necessary, etc).
  3. Use a String key instead of a hash where the String is something like: “fontFamily:fontSize:isBold” where there is no concern about collisions since the keys are unique.

Of these options, I would say that Option 3 sounds the best to me. I would probably put that String as the toString representation of the CellContext class to separate concerns and delegate responsibility for that to the class itself. So the minimal changes that would fix this look like:

public class CellContext {
  ... // existing implementation
  @Override
  public String toString() {
    return String.format("%s:%d:%s", fontFamily, fontSize, isBold);
  }
}

public class CellContextFactory {
  private final Map<String, CellContext> contexts = new HashMap<>();

  public CellContext getContext(String fontFamily, int fontSize, boolean isBold) {
    var context = new CellContext(fontFamily, fontSize, isBold);
    var cachedContext = context.get(context.toString());
    if (cachedContext != null) {
      return cachedContext;
    }
    context.put(context.toString(), context);
    return context;
  }
}

Did anyone else notice this problem?

2 Likes