I was reviewing #326 when I took a closer look at Dictionaries.asMap()
.
The method as it currently stands seems to unnecessarily leave open the potential for type safety violations. The following code snippet demonstrates the possibility:
Hashtable<Integer, Integer> intMap = new Hashtable<>();
Map<Number, Number> castMap = Dictionaries.asMap(intMap);
castMap.put(1.0, 1.9);
The above code compiles, and allows the caller to put floats into the underlying hashtable even though it was declared to accept only integers. Goodness only knows what kind of issues this might introduce further down the code path.
The fix for this is simple enough: the bounded wildcards on the dictionary need to be removed from asMap()
so that the type parameters on the incoming dictionary exactly:
current:
public static <K, V> Map<K, V> asMap(Dictionary<? extends K, ? extends V> dictionary) {
new:
public static <K, V> Map<K, V> asMap(Dictionary<K, V> dictionary) {
This prevents the above code from compiling.
I'm struggling to the see the use case for including the bounds on the parameter to asMap()
, but I raised this ticket in case someone else sees the need. If there is a genuine use case for it, I suggest that we push the burden of doing the unsafe cast back to the caller, eg:
Hashtable<Integer, Integer> intMap = new Hashtable<>();
Map<Number, Number> castMap = Dictionaries.asMap((Dictionary<Number, Number>)intMap);
castMap.put(1.0, 1.9);
or:
Hashtable<Integer, Integer> intMap = new Hashtable<>();
Map<Number, Number> castMap = (Map<Number, Number>)Dictionaries.asMap(intMap);
castMap.put(1.0, 1.9);
That way, if the caller wants to do something silly like this for whatever reason, at least the potential for compile-time type safety circumvention will be made explicit in their code. With the current API, we are hiding the type safety circumvention behind our own unsafe cast.
Because this is a breaking change (albeit a completely justifiable one IMO), I feel that it would be good to try and squeeze this into the 1.0 release (if possible) by cherry picking it?