Java generics - Map of (typed) maps

I'm working on a (simple) caching so开发者_如何学Pythonlution of sorts, where a service can request a Cache object from a Map of caches. A Cache object works essentially just like a Map, too, with a key and a value and methods to access and store objects.

I came up with the following solution, but as you can see, it contains a cast (because get() can't know what the types of the nested object are supposed to be).

private final  Map<String, Cache<?, ?>> caches = new HashMap<String, Cache<?, ?>>();

public <K, V> Cache<K, V> getOrCreateCache(String identifier) {
    if (caches.containsKey(identifier)) {
        return (Cache<K, V>) caches.get(identifier);
    } else {
        Cache<K, V> cache = new CacheImpl<K, V>();
        caches.put(identifier, cache);
        return cache;
    }
}

private void test() {
    Cache<String, String> strCache = getOrCreateCache("string cache");
    strCache.set("key", "value");
}

Now, my questions:

• Is this a 'safe' approach, as long as classcastexceptions are handled properly? (probably going to catch those and pack them into a custom exception class)
• Is there a 'safe' alternative? One with generics, if at all possible, because I like them and dislike casts.
• (not directly related) Is this threadsafe? I assume not, but then, I'm no threading expert. Is it enough to just make the whole method synchronized, or would that (with half a dozen clients) cause too much overhead / locking? Is there a neat solution for that?

Edit: Woo, lots of answers, thanks! Editing here to describe an oddity I found while actually testing this:

    Cache<String, String> someCache = service.getOrCreateCache(cacheIdentifier);
    someCache.set("asdf", "sdfa");
    Cache<String, Integer> someCacheRetrievedAgain = service.getOrCreateCache(cacheIdentifier);
    System.out.println(someCacheRetrievedAgain.get("asdf")); // prints "sdfa". No errors whatsoever. Odd.

You could create a composite key that consists of your current identifier and two instances of Class (one for key, one for value)

public <K, V> Cache<K, V> getOrCreateCache(String identifier, Class<K> keyClass, Class<V> valueClass) {
  Identifier cacheIdentifier = new Identifier(identifier, keyClass, valueClass);
  // safe cast as we know that this cacheIdentifier must has a Cache<K, V>
  Cache<K, V> cache = (Cache<K, V>) caches.get(identifier);
  if (cache == null) {
    cache = new CacheImpl<K, V>();
    caches.put(cacheIdentifier, cache);
  }
  return cache;
}

/*
 * not the most efficient implementation, but correctly implements hashCode and equals
 * which is all we need
 */
private static class CacheIdentifier extends ArrayList<Object> {
  private CacheIdentifier(String identifier, Class<K> keyClass, Class<V> valueClass) {
    super(3);
    // TODO check for null
    add(identifier);
    add(keyClass);
    add(valueClass);
  }
}

To make this thread safe, use a ConcurrentHashMap instead along with putIfAbsent(..)

On the thread safety question, no it's not thread safe. You should look at ConcurrentHashMap or Google Guava's MapMaker

You can just make the whole method synchornized to make it thread safe. Provided its not called often it will be efficient enough. If you want to make the code safer I suggest you try the following add a runtime check for the types.

public <K, V> Cache<K, V> getOrCreateCache(String identifier, Class<K> kClass, Class<V> vClass) {
    Cache<K, V> cache = (Cache<K, V>) caches.get(identifier);
    if(cache == null)
        caches.put(identifier, cache = new CacheImpl<K, V>(kClass, vClass));
    assert cache.kClass() == kClass;
    assert cache.vClass() == vClass;
    return cache;
}

Is this a 'safe' approach, as long as classcastexceptions are handled properly? (probably going to catch those and pack them into a custom exception class)

Actually, the safest way to handle that would be to create the cache key using the key and value types. Something like this:

public String getCacheKey(Class<?> keyType, Class<?> valueType, String uniqueId){
    return keyType.getName()+"-"+valueType.getName()+"-"+uniqueId;
}

That way you'd be sure that a cache is of the specified type.

Is there a 'safe' alternative? One with generics, if at all possible, because I like them and dislike casts.

Basically: if you don't like unchecked casting, you will have to supply implementation types to all methods.

(not directly related) Is this threadsafe? I assume not, but then, I'm no threading expert. Is it enough to just make the whole method synchronized, or would that (with half a dozen clients) cause too much overhead / locking? Is there a neat solution for that?

Synchronizing the method is awful. Use a ConcurrentHashMap and it's putIfAbsent() method

There was a similar question yesterday. Your solution is not safe as there is nothing about the key that implies the type of the value. In other question the key was Callable<T> and the value was T. Thus a custom map could be made which insured type safety, and prevent the underlying map from being corrupted.