Problem
Casting, instanceof, and @SuppressWarnings(“unchecked”) are noisy. It would be nice to stuff them down into a method where they won’t need to be looked at. CheckedCast.castToMapOf()
is an attempt to do that.
castToMapOf()
is making some assumptions:
- (1) The map can’t be trusted to be homogeneous
- (2) Redesigning to avoid need for casting or instanceof is not viable
- (3) Ensuring type safety in an fail early manner is more important than the performance hit
- (4) Returning
Map<String,String>
is sufficient (rather than returningHashMap<String, String>
) - (5) The key and value type args are not generic (like
HashMap<ArrayList<String>, String>
)
(1), (2) and (3) are symptoms of my work environment, beyond my control. (4) and (5) are compromises I’ve made because I haven’t found good ways to overcome them yet.
(4) Is difficult to overcome because even if a HashMap.class
was passed into a Class<M>
I haven’t been able to figure out how to return a M<K, V>
. So I return a Map<K, V>
.
(5) Is probably an inherent limitation of using Class<T>
. I’d love to hear alternative ideas.
Despite those limitations can you see any problems with this java 1.5 code? Am I making any assumptions I haven’t identified? Is there a better way to do this? If I’m reinventing the wheel please point me to the wheel. 🙂
Usage code block:
public class CheckedCast {
public static final String LS = System.getProperty("line.separator");
public static void main(String[] args) {
// -- Raw maps -- //
Map heterogeneousMap = new HashMap();
heterogeneousMap.put("Hmm", "Well");
heterogeneousMap.put(1, 2);
Map homogeneousMap = new HashMap();
homogeneousMap.put("Hmm", "Well");
// -- Attempts to make generic -- //
//Unsafe, will fail later when accessing 2nd entry
@SuppressWarnings("unchecked") //Doesn't check if map contains only Strings
Map<String, String> simpleCastOfHeteroMap =
(Map<String, String>) heterogeneousMap;
//Happens to be safe. Does nothing to prove claim to be homogeneous.
@SuppressWarnings("unchecked") //Doesn't check if map contains only Strings
Map<String, String> simpleCastOfHomoMap =
(Map<String, String>) homogeneousMap;
//Succeeds properly after checking each item is an instance of a String
Map<String, String> checkedCastOfHomoMap =
castToMapOf(String.class, String.class, homogeneousMap);
//Properly throws ClassCastException
Map<String, String> checkedCastOfHeteroMap =
castToMapOf(String.class, String.class, heterogeneousMap);
//Exception in thread "main" java.lang.ClassCastException:
//Expected: java.lang.String
//Was: java.lang.Integer
//Value: 1
// at checkedcast.CheckedCast.checkCast(CheckedCast.java:14)
// at checkedcast.CheckedCast.castToMapOf(CheckedCast.java:36)
// at checkedcast.CheckedCast.main(CheckedCast.java:96)
}
Methods code block:
/** Check all contained items are claimed types and fail early if they aren't */
public static <K, V> Map<K, V> castToMapOf(
Class<K> clazzK,
Class<V> clazzV,
Map<?, ?> map) {
for ( Map.Entry<?, ?> e: map.entrySet() ) {
checkCast( clazzK, e.getKey() );
checkCast( clazzV, e.getValue() );
}
@SuppressWarnings("unchecked")
Map<K, V> result = (Map<K, V>) map;
return result;
}
/** Check if cast would work */
public static <T> void checkCast(Class<T> clazz, Object obj) {
if ( !clazz.isInstance(obj) ) {
throw new ClassCastException(
LS + "Expected: " + clazz.getName() +
LS + "Was: " + obj.getClass().getName() +
LS + "Value: " + obj
);
}
}
Some reading I found helpful:
Generic factory with unknown implementation classes
Generic And Parameterized Types
I’m also wondering if a TypeReference / super type tokens might help with (4) and (5) and be a better way to approach this problem. If you think so please post an example.
Solution
You should really rewrite the main
method as proper unit tests, with separate test cases, for example:
private static void iterateMapKeysAsStrings(Map<String, ?> map) {
for (String key : map.keySet()) {
// nothing to do, invalid cast will be triggered for wrong type
}
}
@Test(expected = ClassCastException.class)
public void testHeterogeneousMap() {
Map heterogeneousMap = new HashMap();
heterogeneousMap.put("Hmm", "Well");
heterogeneousMap.put(1, 2);
//Unsafe, will fail later when accessing 2nd entry
//Doesn't check if map contains only Strings
@SuppressWarnings("unchecked")
Map<String, String> map = (Map<String, String>) heterogeneousMap;
iterateMapKeysAsStrings(map);
}
@Test
public void testHomogeneousMap() {
Map homogeneousMap = new HashMap();
homogeneousMap.put("Hmm", "Well");
//Happens to be safe. Does nothing to prove claim to be homogeneous.
//Doesn't check if map contains only Strings
Map<String, String> simpleCastMap = (Map<String, String>) homogeneousMap;
iterateMapKeysAsStrings(simpleCastMap);
//Succeeds properly after checking each item is an instance of a String
Map<String, String> safeCastMap = castToMapOf(String.class, String.class, homogeneousMap);
iterateMapKeysAsStrings(safeCastMap);
}
I added a helper method iterateMapKeysAsStrings
to trigger ClassCastException
after an unsafe cast.
For testing an invalid cast with castToMapOf
, you don’t need to save the result in a variable:
@Test(expected = ClassCastException.class)
public void testInvalidCast() {
Map heterogeneousMap = new HashMap();
heterogeneousMap.put("Hmm", "Well");
heterogeneousMap.put(1, 2);
//Properly throws ClassCastException
castToMapOf(String.class, String.class, heterogeneousMap);
}
I couldn’t improve the implementation of castToMapOf
. I tried a few things, but they didn’t work out. The unit tests helped a lot in this: you either get “all green” results, or if something fails, you can pinpoint the problem, without having to read everything including the successful cases.
In comments you wrote that you like the detailed text in the ClassCastException
in the checkCast
method. I don’t really see how this message matters. I understand that if you “test” your code with your original main
method, it’s easy to read. But with unit tests you don’t have to read at all. I think less code is generally better: you could use a shorter message with the LS
variable, without the System.getProperty("line.separator")
.