How do I address unchecked cast warnings?
Asked Answered
D

22

704

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.
Disciplinary answered 3/2, 2009 at 21:57 Comment(5)
If you're using HttpSession like that, check out Brian Goetz's article on the subject: ibm.com/developerworks/library/j-jtp09238.htmlMidi
If an unchecked cast is unavoidable, a good idea is to tightly couple it with something that logically represents it's type (like an enum or even instances of Class<T>), so you can glance at it and know it's safe.Cardoza
Related/dupe: Type safety: Unchecked castLang
possible duplicate of Type safety: Unchecked castGet
I would add, I found I could only add @SuppressWarnings("unchecked") at the method level that contains the offending code. So I broke the code out to a routine where I had to do this. I always thought you could do this immediately above the line in question.Gad
D
114

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?

Disciplinary answered 3/2, 2009 at 22:35 Comment(16)
You can't parameterize it unless you add two Class parameters also. Your signature would be "public static <K,V> HashMap<K,V> checkHash(HashMap<?,?> input, Class<K> keyClass, Class<V> valueClass)". Then you'd use keyClass.isAssignableFrom(key.getClass()) and keyClass.cast(key).Plier
I'm not sure it's worth it; it depends on the context. An alternative would be to declare the method like this: Map<?,?> getItems() and do the cast to String in the client code. Properly use of generics assures that a ClassCastException is never raised in the absence of an explicit cast.Hammon
this stuff is confusing, but i think all you have done is traded ClassCastExceptions for AssertionErrors.Discomfit
I added an answer to this thread; future googlers may wish to refer to my answer: #509576Discomfit
Dude, that's definitely not worth it! Imagine the poor sap who has to come back and modify some code with that mess in there. I don't like suppressing warnings, but I think that's the lesser evil here.Lamphere
It's not just that it's an ugly, confusing, mess (when you can't avoid one copious comments can walk the maintenance programmer through it); iterating over every element in the collection turns the cast from an O(1) to an O(n) operation. This is something that would never be expected and can easily turn into a horrible mystery slowdown.Fabria
@DanNeely you are correct. In general, nobody should ever do this.Disciplinary
@DanNeely compiler warnings exist for a reason, we would all benefit as engineers from not ignoring them.Cirque
Suppose theHash is of compile-time type Hash<String,String>. If you have String s = theHash.get(x), there is a run-time check that the result is in fact a String. Thus, if for some reason theMap does not map Strings to Strings, there will be an exception eventually thrown. It just won't be at the point of the original cast.Methionine
@Disciplinary I think you should update your answer to add some huge warnings saying why this (lovely) generics code should not be used in this context. I think we all agree a big performance hit and a maintenance nightmare are not appropriate here to suppress a build warning.Beginner
If it's absolutely critical that it doesn't generate ClassCastExceptions during retrieval AND you're legitimately concerned that the Map might contain the wrong types then I think that would be the only time it would make sense. As stated, though, you're getting the AssertionErrors up front rather than ClassCastExceptions during retrieval, and I must add to the list of doubts that this would ever be worth it.Laguna
Some comments...the method signature is wrong because it doesn't "cast" a damn thing, it just copies the existing map into a new map. Also, it could probably be refactored to accept any map, and not rely on HashMap itself (i.e. take Map and return Map in the method signature, even if the internal type is HashMap). You don't really need to do the casting or the storage into a new map - if you don't throw an assertion error, then the given map has the right types inside it as of right now. Creating a new map with the generic types is pointless as you could still make it raw and put whatever.Trouper
I work in an environment where this kind of code is demanded of us. The performance hit is nothing in our case but it sure is ugly to look at. So I developed a way to hide this noise: stuff it all down into a generic method. Put it before your loops and you'll be looking at cleaner code. #25618952Joaquinajoash
I see this run-time cast check as a middle ground between compile-time warnings that may be too restrictive in their anticipation of the worst case and the run-time errors that may occur far away from the dangerous modification, as @TheodoreNorvell mentioned. I just learned that using the wildcard ? as a type parameter may relieve from getting a warning during the cast of an instance to a parameterized-type reference. The wildcard's purpose seems to assume that reading Object-like members will be safe and that writing to these members will raise a different compile-time warning.Bellow
HashMap is still a raw type, You(/Eclipse) can generify code using HashMap<?, ?> inputPuzzlement
One more reason why you should never use this method: If someone did class X<K,V> extends HashMap<K,V> you are actually altering the behaviour of the map after the cast, because x.put(k,v) will call HashMap::put instead of X::put. This can lead to issues that are very hard to debug!Chihli
P
614

The obvious answer, of course, is not to do the unchecked cast.

If it's absolutely necessary, then at least try to limit the scope of the @SuppressWarnings annotation. According to its Javadocs, it can go on local variables; this way, it doesn't even affect the entire method.

Example:

@SuppressWarnings("unchecked")
Map<String, String> myMap = (Map<String, String>) deserializeMap();

There is no way to determine whether the Map really should have the generic parameters <String, String>. You must know beforehand what the parameters should be (or you'll find out when you get a ClassCastException). This is why the code generates a warning, because the compiler can't possibly know whether is safe.

Plier answered 3/2, 2009 at 22:7 Comment(8)
+1 for pointing out that it can go on local variables. Eclipse only offers to add it to the whole method...Canty
Eclipse 3.7 (Indigo) has support for adding unchecked to local variables.Stonybroke
@Canty Is that because this only works on newer versions of Java?Zugzwang
@Zugzwang I don't think so, AFAIK this works since annotations exist.Canty
The warning is not just because the compiler does not know that the cast is safe. For example String s = (String) new Object() ; gets no warning, even though the compiler does not know that the cast is safe. The warning is because the compiler (a) does not know that the cast is safe AND (b) will not generate a complete run-time check at the point of the cast. There will be a check that it is a Hashmap, but there will not be a check that it is a HashMap<String,String>.Methionine
Sadly, even though the cast and the warning are for the assignment, the annotation has to go on the variable declaration... So if the declaration and the assignment are in different places (say, outside and inside a 'try' block respectively), Eclipse now generates two warnings: the original unchecked cast, and a new "unnecessary annotation" diagnostic.Layout
A workaround for the annotation needing to accompany the local variable declaration, which may be in a different scope on a different line than the actual cast, is to create a local variable within the scope of the cast specifically to perform the cast on the same line as the declaration. Then assign this variable to the actual variable which is in a different scope. This is the method I used also to suppress the warning on a cast to an instance variable as the annotation can't be applied here either.Speculator
This is also necessary incase of clients which connect to graphql. Because graphql can return polymorphic collections and later in Java code when we try to cast the collection to a List of concrete class then we get the sonar warningCaril
B
186

Unfortunately, there are no great options here. Remember, the goal of all of this is to preserve type safety. "Java Generics" offers a solution for dealing with non-genericized legacy libraries, and there is one in particular called the "empty loop technique" in section 8.2. Basically, make the unsafe cast, and suppress the warning. Then loop through the map like this:

@SuppressWarnings("unchecked")
Map<String, Number> map = getMap();
for (String s : map.keySet());
for (Number n : map.values());

If an unexpected type is encountered, you will get a runtime ClassCastException, but at least it will happen close to the source of the problem.

Brandabrandais answered 3/2, 2009 at 22:46 Comment(4)
Much, much better answer than the one provided by skiphoppy, for multiple reasons: 1) This code is much, much shorter. 2) This code actually throws ClassCastException as expected. 3) This code does not do a full copy of the source map. 4) The loops can be easily wrapped in a separate method used in an assert, which would easily remove the performance hit in production code.Padauk
Isn't there a possibility that the Java compiler or the JIT compiler will decide that the results of this code is not being used and "optimize" it by not compiling it?Basso
It's not really dead code if it can potentially throw an exception. I don't know enough about the JIT compilers in use today to guarantee that none of them would mess this up, but I feel fairly confident in saying that they're not supposed to.Martainn
This still doesn't guarantee type safety as the same map is still being used. It might have originally been defined as Map<Object,Object> that just happens to have Strings and Numbers in and then later on if a Boolean is added then the user of this code will get a confusing and rather hard to trace surprise. The only way to guarantee type safety is to copy it into a new map with the requested type that guarantees whats allowed to go into it.Abrupt
D
114

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?

Disciplinary answered 3/2, 2009 at 22:35 Comment(16)
You can't parameterize it unless you add two Class parameters also. Your signature would be "public static <K,V> HashMap<K,V> checkHash(HashMap<?,?> input, Class<K> keyClass, Class<V> valueClass)". Then you'd use keyClass.isAssignableFrom(key.getClass()) and keyClass.cast(key).Plier
I'm not sure it's worth it; it depends on the context. An alternative would be to declare the method like this: Map<?,?> getItems() and do the cast to String in the client code. Properly use of generics assures that a ClassCastException is never raised in the absence of an explicit cast.Hammon
this stuff is confusing, but i think all you have done is traded ClassCastExceptions for AssertionErrors.Discomfit
I added an answer to this thread; future googlers may wish to refer to my answer: #509576Discomfit
Dude, that's definitely not worth it! Imagine the poor sap who has to come back and modify some code with that mess in there. I don't like suppressing warnings, but I think that's the lesser evil here.Lamphere
It's not just that it's an ugly, confusing, mess (when you can't avoid one copious comments can walk the maintenance programmer through it); iterating over every element in the collection turns the cast from an O(1) to an O(n) operation. This is something that would never be expected and can easily turn into a horrible mystery slowdown.Fabria
@DanNeely you are correct. In general, nobody should ever do this.Disciplinary
@DanNeely compiler warnings exist for a reason, we would all benefit as engineers from not ignoring them.Cirque
Suppose theHash is of compile-time type Hash<String,String>. If you have String s = theHash.get(x), there is a run-time check that the result is in fact a String. Thus, if for some reason theMap does not map Strings to Strings, there will be an exception eventually thrown. It just won't be at the point of the original cast.Methionine
@Disciplinary I think you should update your answer to add some huge warnings saying why this (lovely) generics code should not be used in this context. I think we all agree a big performance hit and a maintenance nightmare are not appropriate here to suppress a build warning.Beginner
If it's absolutely critical that it doesn't generate ClassCastExceptions during retrieval AND you're legitimately concerned that the Map might contain the wrong types then I think that would be the only time it would make sense. As stated, though, you're getting the AssertionErrors up front rather than ClassCastExceptions during retrieval, and I must add to the list of doubts that this would ever be worth it.Laguna
Some comments...the method signature is wrong because it doesn't "cast" a damn thing, it just copies the existing map into a new map. Also, it could probably be refactored to accept any map, and not rely on HashMap itself (i.e. take Map and return Map in the method signature, even if the internal type is HashMap). You don't really need to do the casting or the storage into a new map - if you don't throw an assertion error, then the given map has the right types inside it as of right now. Creating a new map with the generic types is pointless as you could still make it raw and put whatever.Trouper
I work in an environment where this kind of code is demanded of us. The performance hit is nothing in our case but it sure is ugly to look at. So I developed a way to hide this noise: stuff it all down into a generic method. Put it before your loops and you'll be looking at cleaner code. #25618952Joaquinajoash
I see this run-time cast check as a middle ground between compile-time warnings that may be too restrictive in their anticipation of the worst case and the run-time errors that may occur far away from the dangerous modification, as @TheodoreNorvell mentioned. I just learned that using the wildcard ? as a type parameter may relieve from getting a warning during the cast of an instance to a parameterized-type reference. The wildcard's purpose seems to assume that reading Object-like members will be safe and that writing to these members will raise a different compile-time warning.Bellow
HashMap is still a raw type, You(/Eclipse) can generify code using HashMap<?, ?> inputPuzzlement
One more reason why you should never use this method: If someone did class X<K,V> extends HashMap<K,V> you are actually altering the behaviour of the map after the cast, because x.put(k,v) will call HashMap::put instead of X::put. This can lead to issues that are very hard to debug!Chihli
S
52

In Eclipse Preferences, Go to Java->Compiler->Errors/Warnings->Generic types and check the Ignore unavoidable generic type problems check-box.

This satisfies the intent of the question, i.e.

I'd like to avoid Eclipse warnings...

if not the spirit.

Skyway answered 21/10, 2011 at 17:40 Comment(1)
Ah, thanks for this :) I was getting a "uses unchecked or unsafe operations." error in javac, but adding @SuppressWarnings("unchecked") made Eclipse unhappy, claiming the suppression was unnecessary. Unchecking this box makes Eclipse and javac behave the same, which is what I wanted. Explicitly suppressing the warning in the code is much clearer than suppressing it everywhere inside Eclipse.Bump
C
29

You can create a utility class like the following, and use it to suppress the unchecked warning.

public class Objects {

    /**
     * Helps to avoid using {@code @SuppressWarnings({"unchecked"})} when casting to a generic type.
     */
    @SuppressWarnings({"unchecked"})
    public static <T> T uncheckedCast(Object obj) {
        return (T) obj;
    }
}

You can use it as follows:

import static Objects.uncheckedCast;
...

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

Some more discussion about this is here: http://cleveralias.blogs.com/thought_spearmints/2006/01/suppresswarning.html

Cherian answered 3/2, 2009 at 22:54 Comment(4)
not downvoting, but the wrapper adds precisely nothing over just suppressing the warning.Discomfit
+1 as this solution does not waste precious code lines.Brewing
@ErikE Too much. Much more expensive bigger and higher-resolution monitors to give room to all those wasted lines, a bigger desk to put all those bigger monitors onto, a bigger room to put the bigger desk into, and an insightful boss ..Brewing
@ErikE Scrollbars, for vi? Are you kidding?Brewing
D
23

This stuff is hard, but here are my current thoughts:

If your API returns Object, then there's nothing you can do -- no matter what, you will be blindly casting the object. You let Java throw ClassCastExceptions, or you can check each element yourself and throw Assertions or IllegalArgumentExceptions or some such, but these runtime checks are all equivalent. You have to suppress the compile time unchecked cast no matter what you do at runtime.

I'd just prefer to blind cast and let the JVM perform its runtime check for me since we "know" what the API should return, and are usually willing to assume that the API works. Use generics everywhere above the cast, if you need them. You aren't really buying anything there since you still have the single blind cast, but at least you can use generics from there on up so the JVM can help you avoid blind casts in other pieces of your code.

In this particular case, presumably you can see the call to SetAttribute and see the type is going in, so just blind-casting the type to same on the way out is not immoral. Add a comment referencing the SetAttribute and be done with it.

Discomfit answered 1/3, 2011 at 23:4 Comment(0)
S
18

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);
Stormy answered 13/8, 2015 at 10:57 Comment(0)
E
13

In the HTTP Session world you can't really avoid the cast, since the API is written that way (takes and returns only Object).

With a little bit of work you can easily avoid the unchecked cast, 'though. This means that it will turn into a traditional cast giving a ClassCastException right there in the event of an error). An unchecked exception could turn into a CCE at any point later on instead of the point of the cast (that's the reason why it's a separate warning).

Replace the HashMap with a dedicated class:

import java.util.AbstractMap;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

public class Attributes extends AbstractMap<String, String> {
    final Map<String, String> content = new HashMap<String, String>();

    @Override
    public Set<Map.Entry<String, String>> entrySet() {
        return content.entrySet();
    }

    @Override
    public Set<String> keySet() {
        return content.keySet();
    }

    @Override
    public Collection<String> values() {
        return content.values();
    }

    @Override
    public String put(final String key, final String value) {
        return content.put(key, value);
    }
}

Then cast to that class instead of Map<String,String> and everything will be checked at the exact place where you write your code. No unexpected ClassCastExceptions later on.

Eaglet answered 4/2, 2009 at 16:59 Comment(1)
This is really helpful answer.Collyer
D
10

In Android Studio if you want to disable inspection you can use:

//noinspection unchecked
Map<String, String> myMap = (Map<String, String>) deserializeMap();
Dannadannel answered 1/9, 2017 at 20:22 Comment(0)
C
8

In this particular case, I would not store Maps into the HttpSession directly, but instead an instance of my own class, which in turn contains a Map (an implementation detail of the class). Then you can be sure that the elements in the map are of the right type.

But if you anyways want to check that the contents of the Map are of right type, you could use a code like this:

public static void main(String[] args) {
    Map<String, Integer> map = new HashMap<String, Integer>();
    map.put("a", 1);
    map.put("b", 2);
    Object obj = map;

    Map<String, Integer> ok = safeCastMap(obj, String.class, Integer.class);
    Map<String, String> error = safeCastMap(obj, String.class, String.class);
}

@SuppressWarnings({"unchecked"})
public static <K, V> Map<K, V> safeCastMap(Object map, Class<K> keyType, Class<V> valueType) {
    checkMap(map);
    checkMapContents(keyType, valueType, (Map<?, ?>) map);
    return (Map<K, V>) map;
}

private static void checkMap(Object map) {
    checkType(Map.class, map);
}

private static <K, V> void checkMapContents(Class<K> keyType, Class<V> valueType, Map<?, ?> map) {
    for (Map.Entry<?, ?> entry : map.entrySet()) {
        checkType(keyType, entry.getKey());
        checkType(valueType, entry.getValue());
    }
}

private static <K> void checkType(Class<K> expectedType, Object obj) {
    if (!expectedType.isInstance(obj)) {
        throw new IllegalArgumentException("Expected " + expectedType + " but was " + obj.getClass() + ": " + obj);
    }
}
Cherian answered 3/2, 2009 at 23:19 Comment(2)
Awesome; I think I can combine that with my answer to parameterize it and avoid having to suppress warnings altogether!Disciplinary
+1 probably best recipe (easy to understand and maintain) to do it safely with runtime checksBrewing
S
8

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.

Segregate answered 11/8, 2011 at 13:53 Comment(0)
K
6

Warning suppression is not a solution. You should not be doing two level casting in one statement.

HashMap<String, String> getItems(javax.servlet.http.HttpSession session) {

    // first, cast the returned Object to generic HashMap<?,?>
    HashMap<?, ?> theHash = (HashMap<?, ?>)session.getAttribute("attributeKey");

    // next, cast every entry of the HashMap to the required type <String, String>
    HashMap<String, String> returingHash = new HashMap<>();
    for (Entry<?, ?> entry : theHash.entrySet()) {
        returingHash.put((String) entry.getKey(), (String) entry.getValue());
    }
    return returingHash;
}
Kata answered 9/3, 2015 at 23:43 Comment(2)
His five-year-old question? Do you need to do that much work? Given Java has type erasure the second hashmap should be identical to the first at runtime; I think it'd be more efficient and avoid the copy if you just iterated through the entries and verified that they were all instances of strings. Or, TBH, inspect the source of the servlet JAR you're using and verify it only ever puts strings.Ushijima
To this day I am seeing this warning in projects. His problem was not verification of the type, but a warning caused by a "put" into an uncasted map.Kata
C
2

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.

Croupier answered 3/2, 2009 at 22:5 Comment(1)
I'm pretty sure that switching to Map in this case would not change anything, but thanks for the programming tip, which may change the way I do some things, for the better. The source of the object is an API I have no control over (code added).Disciplinary
D
2

If I have to use an API that doesn't support Generics.. I try and isolate those calls in wrapper routines with as few lines as possible. I then use the SuppressWarnings annotation and also add the type-safety casts at the same time.

This is just a personal preference to keep things as neat as possible.

Dett answered 3/2, 2009 at 22:47 Comment(0)
L
2

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);
    }
Longevous answered 14/10, 2010 at 9:28 Comment(1)
key.isAssignableFrom(e.getKey().getClass()) can be written as key.isInstance(e.getKey())Weise
D
1

A quick guess if you post your code can say for sure but you might have done something along the lines of

HashMap<String, Object> test = new HashMap();

which will produce the warning when you need to do

HashMap<String, Object> test = new HashMap<String, Object>();

it might be worth looking at

Generics in the Java Programming Language

if your unfamiliar with what needs to be done.

Dyewood answered 3/2, 2009 at 22:4 Comment(2)
Unfortunately it's not that easy a situation. Code added.Disciplinary
I came here looking for an answer to a slightly different problem: and you told me exactly what I needed! Thanks!Koressa
M
1

Almost every problem in Computer Science can be solved by adding a level of indirection*, or something.

So introduce a non-generic object that is of a higher-level that a Map. With no context it isn't going to look very convincing, but anyway:

public final class Items implements java.io.Serializable {
    private static final long serialVersionUID = 1L;
    private Map<String,String> map;
    public Items(Map<String,String> map) {
        this.map = New.immutableMap(map);
    }
    public Map<String,String> getMap() {
        return map;
    }
    @Override public String toString() {
        return map.toString();
    }
}

public final class New {
    public static <K,V> Map<K,V> immutableMap(
        Map<? extends K, ? extends V> original
    ) {
        // ... optimise as you wish...
        return Collections.unmodifiableMap(
            new HashMap<String,String>(original)
        );
    }
}

static Map<String, String> getItems(HttpSession session) {
    Items items = (Items)
        session.getAttribute("attributeKey");
    return items.getMap();
}

*Except too many levels of indirection.

Midi answered 4/2, 2009 at 15:46 Comment(1)
The quote is attributed to the late Professor David Wheeler. en.wikipedia.org/wiki/…Refractor
B
1

Here's one way I handle this when I override the equals() operation.

public abstract class Section<T extends Section> extends Element<Section<T>> {
    Object attr1;

    /**
    * Compare one section object to another.
    *
    * @param obj the object being compared with this section object
    * @return true if this section and the other section are of the same
    * sub-class of section and their component fields are the same, false
    * otherwise
    */       
    @Override
    public boolean equals(Object obj) {
        if (obj == null) {
            // this exists, but obj doesn't, so they can't be equal!
            return false;
        }

        // prepare to cast...
        Section<?> other;

        if (getClass() != obj.getClass()) {
            // looks like we're comparing apples to oranges
            return false;
        } else {
            // it must be safe to make that cast!
            other = (Section<?>) obj;
        }

        // and then I compare attributes between this and other
        return this.attr1.equals(other.attr1);
    }
}

This seems to work in Java 8 (even compiled with -Xlint:unchecked)

Brumley answered 30/1, 2015 at 0:15 Comment(0)
O
0

If you are sure that the type returned by session.getAttribute() is HashMap then you can not typecast to that exact type, but rely on only checking the generic HashMap

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

Eclipse will then surprise warnings, but of course this can lead to runtime errors that can be hard to debug. I use this approach in not operation-critical contexts only.

Oriya answered 12/6, 2010 at 12:52 Comment(0)
E
0

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;
    }


}
Extravasation answered 21/3, 2011 at 19:9 Comment(1)
the fact that you don't have comment privileges do not allow you to edit others' answers to add your comments; you edit others' answers to improve them in formatting, syntax, ..., not to add your opinion on them. When you'll reach 50 rep you'll be able to comment everywhere, in the meantime I'm quite sure you can resist (or, if you really can't, write your comments to existing answers in your post). (note for others: I write this because I saw - and rejected - his proposed comments-edits to other posts in the moderation tools)Matrilineal
T
-1

Just typecheck it before you cast it.

Object someObject = session.getAttribute("attributeKey");
if(someObject instanceof HashMap)
HashMap<String, String> theHash = (HashMap<String, String>)someObject;  

And for anyone asking, it's quite common to receive objects where you aren't sure of the type. Plenty of legacy "SOA" implementations pass around various objects that you shouldn't always trust. (The horrors!)

EDIT Changed the example code once to match the poster's updates, and following some comments I see that instanceof doesn't play nicely with generics. However changing the check to validate the outer object seems to play well with the commandline compiler. Revised example now posted.

Tenner answered 3/2, 2009 at 22:16 Comment(1)
Unfortunately, generics render that impossible. It's not just a HashMap, it's a HashMap with type information. And if I eliminate that information, I'll just push the warnings to elsewhere.Disciplinary
C
-6

Solution: Disable this warning in Eclipse. Don't @SuppressWarnings it, just disable it completely.

Several of the "solutions" presented above are way out of line, making code unreadable for the sake of suppressing a silly warning.

Candlepower answered 28/2, 2013 at 20:27 Comment(1)
May I ask why? globally disabling a warning will hide other places where this issue is real. adding a @SuppressWarnings doesn't make the code unreadable at all.Gnathonic

© 2022 - 2024 — McMap. All rights reserved.