Map with different types for values

I am trying to implements a map (that can work as a cache) that has strings as keys and values of different types. In my real case once set the key/values in the map will not change.

I used these classes, I take as example the Item 29 on Effective Java 2nd edition.

This is the class for values:

final class CachedValue {

    private Object value;

    public  void setValue(Class type, T instance){

        if(type == null){

            log.error("type is null");

            throw new NullPointerException("Type is null");
        }

        value = type.cast(instance);
    }

    public  T getValue(Class type){

        return type.cast(value);
    }
}

This for the map that works like a cache:

public final class Cache {

            private final Map map = Maps.newHashMap();

            public  void put(String key, Class type, T value){

                CachedValue cachedValue = new CachedValue();

                cachedValue.setValue(type, value);

                map.put(key, cachedValue);
            }

            public  T get(String key, Class type){

                return map.get(key).getValue(type);
            }

            public boolean contains(String key){

                return map.containsKey(key);
            }

      }

Can someone review my code and tell me if it's right or if it can be simplify? I don't need it to be thread-safe.

No Type Safety

Currently your classes provide no type safety:

Cache cache = new Cache();
cache.put("my key", Integer.class, 5);
// ...
String cached = cache.get("my key", String.class);

This compiles but fails at runtime with a ClassCastException thrown by CachedValue.getValue.

What's The Key?

The reason why this can happen is that your class is not clear on the role of the type given to put and get. Considering the internal map, the type is clearly part of the value. But get uses it as it were part of the key. This makes it possible to request a value which is there (considering the string key in the map) but of the wrong type.

Compare this to the typesafe heterogeneous container as defined in Effective Java. There the type is part of the key (actually the only part) so if something is found in the map, it must be of the correct type. It is hence safe to cast to that type.

So in order to make your classes type safe you have to decide on what exactly the key is.

String

If the key is only the string, I see no way to provide type safety. To keep the API from surprising its users you should then simply return Object. This effectively degrades your cache to a map from String to Object, which I guess is not your intention.

String & Class

The other possiblitiy is to make the type a full memeber of the key. There are several ways to implement this and I only present one. It uses nested maps which define this mapping:

String -> (Class -> Object)

This can be interpreted as

(String, Class) -> Object

so the type is part of the key.

public class Cache {

  private final Map, Object>> internalMap;

  public Cache() {
    internalMap = new HashMap<>();
  }

  public  void put(String textKey, Class typeKey, T chachedValue) {
    Map, Object> mapForTextKey = getMapForTextKey(textKey);
    mapForTextKey.put(typeKey, chachedValue);
  }

  private Map, Object> getMapForTextKey(String textKey) {
    if (!internalMap.containsKey(textKey))
      internalMap.put(textKey, new HashMap<>());
    return internalMap.get(textKey);
  }

  public  T get(String textKey, Class typeKey) {
    Object untypedCachedValue = getUntyped(textKey, typeKey);
    // the cast can not fail because:
    // - if there is no value for those keys, the argument is null,
    //      which can always be cast
    // - if there is a value for those keys, `put` made sure that
    //      its type matches the type used as a key
    return typeKey.cast(untypedCachedValue);
  }

  private Object getUntyped(String textKey, Class typeKey) {
    if (internalMap.containsKey(textKey))
      return internalMap.get(textKey).get(typeKey);
    else
      return null;
  }

}

Improvements

My proposal above is only the barebones of an implementation. There is a lot of room for improvements. Some of them are:

• handling null aftert answering these questions:

• should it be valid for keys? (please not!)

• should it be valid for values? (also rather not; doesn't seem to make a lot of sense for a cache)

• let get return an Optional

• use Map.computeIfAbsent for more concise code

• or use Guava's Table instead of the nested maps

• designing the class for extensibility or making it final

Furhter Comments After The Update

Looks much better. :) These comments are mostly nitpicking...

Comments regarding the code:

• About null:

• @NotNull is nice but as far as I know it's purely for tools. To have a runtime guarantee you should add Objects.requireNonNull. On the other hand, you use the HashBasedTable so you're fine - it's documentation says: "Null row keys, columns keys, and values are not supported.". If you rely on that, you should declare table as that type and make sure do write a comment explaining this unusual decision (normally you declare the interface as you did).

• I'd also make typeKey not-nullable.

• I like the use of Optional (see here how much). I recommend to use Java 8's version because I expect it to become more common than guava's version.

• Since Table.getis so concise, getUntyped looks a little pointless now. I would remove the method.

• I like logging the calls. For symmetry I would also add it to put.

On style:

• You are very liberal with empty lines. I'd recommend to not do that and only insert an empty line to separate distinct blocks. There seems to be little need for that in the code above.

StackExchange Code Review Q#68668, answer score: 8