Cast a raw map to a generic map using a method, cleanly and safely in a fail early manner

Question

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) ReturningMap<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.

Answer 1

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").