How do I address unchecked cast warnings?

Question

Eclipse is giving me a warning of the following form:

Type safety: Unchecked cast from Object to HashMap

This is from a call to an API that I have no control over which returns Object:

HashMap<String, String> getItems(javax.servlet.http.HttpSession session) {
  HashMap<String, String> theHash = (HashMap<String, String>)session.getAttribute("attributeKey");
  return theHash;
}

I'd like to avoid Eclipse warnings, if possible, since theoretically they indicate at least a potential code problem. I haven't found a good way to eliminate this one yet, though. I can extract the single line involved out to a method by itself and add @SuppressWarnings("unchecked") to that method, thus limiting the impact of having a block of code where I ignore warnings. Any better options? I don't want to turn these warnings off in Eclipse.

Before I came to the code, it was simpler, but still provoked warnings:

HashMap getItems(javax.servlet.http.HttpSession session) {
  HashMap theHash = (HashMap)session.getAttribute("attributeKey");
  return theHash;
}

Problem was elsewhere when you tried to use the hash you'd get warnings:

HashMap items = getItems(session);
items.put("this", "that");

Type safety: The method put(Object, Object) belongs to the raw type HashMap.  References to generic type HashMap<K,V> should be parameterized.

Answer 1

The Objects.Unchecked utility function in the answer above by Esko Luontola is a great way to avoid program clutter.

If you don't want the SuppressWarnings on an entire method, Java forces you to put it on a local. If you need a cast on a member it can lead to code like this:

@SuppressWarnings("unchecked")
Vector<String> watchedSymbolsClone = (Vector<String>) watchedSymbols.clone();
this.watchedSymbols = watchedSymbolsClone;

Using the utility is much cleaner, and it's still obvious what you are doing:

this.watchedSymbols = Objects.uncheckedCast(watchedSymbols.clone());

NOTE: I feel its important to add that sometimes the warning really means you are doing something wrong like :

ArrayList<Integer> intList = new ArrayList<Integer>();
intList.add(1);
Object intListObject = intList; 

 // this line gives an unchecked warning - but no runtime error
ArrayList<String> stringList  = (ArrayList<String>) intListObject;
System.out.println(stringList.get(0)); // cast exception will be given here

What the compiler is telling you is that this cast will NOT be checked at runtime, so no runtime error will be raised until you try to access the data in the generic container.

Answer 2

Two ways, one which avoids the tag completely, the other using a naughty but nice utility method.
The problem is pre-genericised Collections...
I believe the rule of thumb is: "cast objects one thing at a time" - what this means when trying to use raw classes in a genericised world is that because you don't know what is in this Map<?, ?> (and indeed the JVM might even find that it isn't even a Map!), it obvious when you think about it that you can't cast it. If you had a Map<String, ?> map2 then HashSet<String> keys = (HashSet<String>)map2.keySet() does not give you a warning, despite this being an "act of faith" for the compiler (because it might turn out to be a TreeSet)... but it is only a single act of faith.

PS to the objection that iterating as in my first way "is boring" and "takes time", the answer is "no pain no gain": a genericised collection is guaranteed to contain Map.Entry<String, String>s, and nothing else. You have to pay for this guarantee. When using generics systematically this payment, beautifully, takes the form of coding compliance, not machine time!
One school of thought might say that you should set Eclipse's settings to make such unchecked casts errors, rather than warnings. In that case you would have to use my first way.

package scratchpad;

import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Vector;

public class YellowMouse {

    // First way

    Map<String, String> getHashMapStudiouslyAvoidingSuppressTag(HttpSession session) {
      Map<?, ?> theHash = (Map<?, ?>)session.getAttribute("attributeKey");

      Map<String, String> yellowMouse = new HashMap<String, String>();
      for( Map.Entry<?, ?> entry : theHash.entrySet() ){
        yellowMouse.put( (String)entry.getKey(), (String)entry.getValue() );
      }

      return yellowMouse;
    }


    // Second way

    Map<String, String> getHashMapUsingNaughtyButNiceUtilityMethod(HttpSession session) {
      return uncheckedCast( session.getAttribute("attributeKey") );
    }


    // NB this is a utility method which should be kept in your utility library. If you do that it will
    // be the *only* time in your entire life that you will have to use this particular tag!!

    @SuppressWarnings({ "unchecked" })
    public static synchronized <T> T uncheckedCast(Object obj) {
        return (T) obj;
    }


}

Answer 3

I may have misunderstood the question(an example and a couple of surrounding lines would be nice), but why don't you always use an appropriate interface (and Java5+)? I see no reason why you would ever want to cast to a HashMap instead of a Map<KeyType,ValueType>. In fact, I can't imagine any reason to set the type of a variable to HashMap instead of Map.

And why is the source an Object? Is it a parameter type of a legacy collection? If so, use generics and specify the type you want.

Answer 4

Here is a shortened example that avoids the "unchecked cast" warning by employing two strategies mentioned in other answers.

1. Pass down the Class of the type of interest as a parameter at runtime (Class<T> inputElementClazz). Then you can use: inputElementClazz.cast(anyObject);

2. For type casting of a Collection, use the wildcard ? instead of a generic type T to acknowledge that you indeed do not know what kind of objects to expect from the legacy code (Collection<?> unknownTypeCollection). After all, this is what the "unchecked cast" warning wants to tell us: We cannot be sure that we get a Collection<T>, so the honest thing to do is to use a Collection<?>. If absolutely needed, a collection of a known type can still be built (Collection<T> knownTypeCollection).

The legacy code interfaced in the example below has an attribute "input" in the StructuredViewer (StructuredViewer is a tree or table widget, "input" is the data model behind it). This "input" could be any kind of Java Collection.

public void dragFinished(StructuredViewer structuredViewer, Class<T> inputElementClazz) {
    IStructuredSelection selection = (IStructuredSelection) structuredViewer.getSelection();
    // legacy code returns an Object from getFirstElement,
    // the developer knows/hopes it is of type inputElementClazz, but the compiler cannot know
    T firstElement = inputElementClazz.cast(selection.getFirstElement());

    // legacy code returns an object from getInput, so we deal with it as a Collection<?>
    Collection<?> unknownTypeCollection = (Collection<?>) structuredViewer.getInput();

    // for some operations we do not even need a collection with known types
    unknownTypeCollection.remove(firstElement);

    // nothing prevents us from building a Collection of a known type, should we really need one
    Collection<T> knownTypeCollection = new ArrayList<T>();
    for (Object object : unknownTypeCollection) {
        T aT = inputElementClazz.cast(object);
        knownTypeCollection.add(aT);
        System.out.println(aT.getClass());
    }

    structuredViewer.refresh();
}

Naturally, the code above can give runtime errors if we use the legacy code with the wrong data types (e.g. if we set an array as the "input" of the StructuredViewer instead of a Java Collection).

Example of calling the method:

dragFinishedStrategy.dragFinished(viewer, Product.class);

Answer 5

Take this one, it's much faster than creating a new HashMap, if it's already one, but still secure, as each element is checked against it's type...

@SuppressWarnings("unchecked")
public static <K, V> HashMap<K, V> toHashMap(Object input, Class<K> key, Class<V> value) {
       assert input instanceof Map : input;

       for (Map.Entry<?, ?> e : ((HashMap<?, ?>) input).entrySet()) {
           assert key.isAssignableFrom(e.getKey().getClass()) : "Map contains invalid keys";
           assert value.isAssignableFrom(e.getValue().getClass()) : "Map contains invalid values";
       }

       if (input instanceof HashMap)
           return (HashMap<K, V>) input;
       return new HashMap<K, V>((Map<K, V>) input);
    }

Answer 6

Wow; I think I figured out the answer to my own question. I'm just not sure it's worth it! :)

The problem is the cast isn't checked. So, you have to check it yourself. You can't just check a parameterized type with instanceof, because the parameterized type information is unavailable at runtime, having been erased at compile time.

But, you can perform a check on each and every item in the hash, with instanceof, and in doing so, you can construct a new hash that is type-safe. And you won't provoke any warnings.

Thanks to mmyers and Esko Luontola, I've parameterized the code I originally wrote here, so it can be wrapped up in a utility class somewhere and used for any parameterized HashMap. If you want to understand it better and aren't very familiar with generics, I encourage viewing the edit history of this answer.

public static <K, V> HashMap<K, V> castHash(HashMap input,
                                            Class<K> keyClass,
                                            Class<V> valueClass) {
  HashMap<K, V> output = new HashMap<K, V>();
  if (input == null)
      return output;
  for (Object key: input.keySet().toArray()) {
    if ((key == null) || (keyClass.isAssignableFrom(key.getClass()))) {
        Object value = input.get(key);
        if ((value == null) || (valueClass.isAssignableFrom(value.getClass()))) {
            K k = keyClass.cast(key);
            V v = valueClass.cast(value);
            output.put(k, v);
        } else {
            throw new AssertionError(
                "Cannot cast to HashMap<"+ keyClass.getSimpleName()
                +", "+ valueClass.getSimpleName() +">"
                +", value "+ value +" is not a "+ valueClass.getSimpleName()
            );
        }
    } else {
        throw new AssertionError(
            "Cannot cast to HashMap<"+ keyClass.getSimpleName()
            +", "+ valueClass.getSimpleName() +">"
            +", key "+ key +" is not a " + keyClass.getSimpleName()
        );
    }
  }
  return output;
}

That's a lot of work, possibly for very little reward... I'm not sure if I'll use it or not. I'd appreciate any comments as to whether people think it's worth it or not. Also, I'd appreciate improvement suggestions: is there something better I can do besides throw AssertionErrors? Is there something better I could throw? Should I make it a checked Exception?