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

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) ReturningMapis sufficient (rather than returningHashMap)

• (5) The key and value type args are not generic (like HashMap, 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 I haven't been able to figure out how to return a M. So I return a Map.

(5) Is probably an inherent limitation of using Class. 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 simpleCastOfHeteroMap =
(Map) heterogeneousMap;

//Happens to be saf

You should really rewrite the main method as proper unit tests, with separate test cases, for example:

private static void iterateMapKeysAsStrings(Map 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 map = (Map) 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 simpleCastMap = (Map) homogeneousMap;
    iterateMapKeysAsStrings(simpleCastMap);

    //Succeeds properly after checking each item is an instance of a String
    Map 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").

StackExchange Code Review Q#61756, answer score: 6