34

I've got the following code:

private HashMap<Class<?>, HashMap<Entity, ? extends Component>> m_componentStores;

public <T extends Component> T getComponent(Entity e, Class<T> exampleClass)
{
    HashMap<Entity, ? extends Component> store = m_componentStores.get(exampleClass);

    T result = (T)store.get(e);

    if (result == null)
    {
        throw new IllegalArgumentException( "GET FAIL: "+e+" does not possess Component of class\nmissing: "+exampleClass );
    }

    return result;
}

When I compile, it shows that T result = (T)store.get(e) has an unchecked cast. 

Type safety: Unchecked cast from capture#2-of ? extends Component to T

What am I missing to prevent this warning from appearing?

Brad
  • 9,113
  • 10
  • 44
  • 68

3 Answers3

50

Class.cast is what you want. Well, you might consider not using reflection.

Change the line:

T result = (T)store.get(e);

to:

T result = exampleClass.cast(store.get(e));
Tom Hawtin - tackline
  • 145,806
  • 30
  • 211
  • 305
  • +1, always better to keep the ClassCastException in the library code IMO. It's not strictly necessary though if you can prove to yourself that your library doesn't make a type mistake (i.e. `setComponent` works properly and symmetrically). Then a suppress warnings will do. – Mark Peters Dec 08 '10 at 16:46
  • 4
    @Mark Peters In most issues where programmers convince themselves, they are typically wrong. – Tom Hawtin - tackline Dec 08 '10 at 17:03
  • 3
    I don't think that's true at all of library designers, and if it is they shouldn't be writing libraries. There are examples of unchecked casts in the API. `Collections.emptyList()` comes to mind. – Mark Peters Dec 08 '10 at 17:49
  • How we do with generics ? required: MyClass found: MyClass – Aguid Oct 07 '15 at 07:42
  • @AyadiAkrem You can't check parameterised types. (If the object is of an unparameterised subclass you could cast to that. Or the object could be referenced by another object of an unparameterised type.) – Tom Hawtin - tackline Oct 07 '15 at 12:06
  • very good, finally being able to remove all such unnecessary "unchecked" annotations, thx! – Aquarius Power Jan 03 '16 at 22:19
  • This answer would be more helpful if it explained *why* `Class.cast` is what we want in this situation. E.g. because it throws an exception if the object (is not null and) is not assignable to the type T, whereas the `(T)` cast fails silently? – LarsH Apr 06 '18 at 15:56
  • 2
    And would be more helpful if it showed how to do Class.cast for a generic type (which you cannot). Or provide some insight into how Class.cast can be remotely applicable in such situations. If there were a common type from which a Class.cast could be made, would it not be much more sensible to use "? extends BaseClass" instead of BaseClass.cast, since that enforces the base class requirement for arguments as well as internal and returned values? – Robin Davies May 23 '18 at 23:04
  • 1
    @RobinDavies That's way out of scope for the questions asked. (If you do require that sort of thing, then you have gone way off piste. As for using wildcards: it's a `Class` object, so not directly relevant. On the method type parameter, it would only obfuscate things. It's not necessary to specify the type argument when calling the method, and a wildcard will be captured just fine.) – Tom Hawtin - tackline May 25 '18 at 07:55
  • @TomHawtin The problem with your answer is that there is no conceivable "exampleClass" to use that improves upon the situation in any imaginable way. If you were to provide a concrete example for a case where an unchecked cast warning has been given, you'll see why. Wasted time trying to apply this advice concretely, and it does not work. – Robin Davies May 28 '18 at 17:18
  • 1
    @RobinDavies `exampleClass` is from the original question. – Tom Hawtin - tackline May 29 '18 at 01:22
16

Write @SuppressWarnings("unchecked") above the Cast statement:

@SuppressWarnings("unchecked")
T result = (T)store.get(e);

And add a explanatory statement why it is safe to ignore the warning.

raffian
  • 31,267
  • 26
  • 103
  • 174
Ralph
  • 118,862
  • 56
  • 287
  • 383
  • 2
    It is not safe to ignore. Let's assume a simple example: `abstract class Animal { public abstract void makeNoice(); }`, `class Cat extends Animal { }` and `class Dog extends Animal {}`. `Dog` and `Cat` are both `Animal` (sure they are). Now: `class SomeAnimalUtils { public static void makeNoise (Animal animal) { Dog dog = (Dog) animal; dog.makeNoice(); } }`. Let's assume that both implement the method correctly. Now `SomeAnimalUtils.makeNoise()` may take a `Cat` because it extends the wanted `Animal` but it cannot be cast to `Dog` and you cat the `ClassCastException`. – Roland Jan 25 '18 at 22:58
  • Well, my example is stupid here but it should demonstrate that you indeed need to handle unchecked casts. To prevent people from giving `Cat` in but only `Dog` (which could be a super class for others like `Terrier` or `Husky`) in. But if someone gives unintended `Cat` in the JVM throws the exception. This is bad because it means the programmer of `SomeAnimalUtils` has not checked the parameter and thrown the exception by hilfself. – Roland Jan 25 '18 at 23:01
  • So you need to pre-check this. To do so, add the following code before the unsafe/unchecked cast: `if (null == animal) { throw new NullPointerException("Parameter 'animal' is null"); } else if (!(animal instanceof Dog)) { throw new ClassCastException("Parameter 'animal'="+animal.toString()+" is not instance of Dog"); }`. Here you also (have to) take care of null-reference check which is allowed by Java language to pass as "objected" parameter (= instance of `Object` or any interface). If you don't take care of null references, you maybe risk an infamous `NPE` here. – Roland Jan 25 '18 at 23:04
  • 1
    Still on topic: A good practice is, to have all public methods check their "objected" parameters for null references and valid data range, e.g. id numbers coming from databases cannot go below `1`, usually. ;-) – Roland Jan 25 '18 at 23:09
  • 1
    @Roland: You are right, that it ignoring a warning should be done with care. The dissension that this cast is save or not depends on the use case of the method and its context, in this case it depends almost on the method that is used to put the value in the map. This method is not listed in the question. Therefore no one but the author can judge this. – Ralph Jan 27 '18 at 12:46
  • @Roland: Unless I'm mistaken, I think you've completely missed the point -- the point being that if (T) is a generic type, the cast occurs WITHOUT throwing a cast exception, even if the object is of the wrong type. (which is why it as an "unchecked cast". Nullability is a completely different topic. 5 whole posts, and you haven't proposed a reasonable way to fix it. – Robin Davies May 23 '18 at 22:58
5

extends in generics doesn't really work that way. T != ? extends Component even though T extends Component. What you have is in fact a wildcard capture, it has a different purpose.

And yes your solution is not type-safe - there is no relation between the two ? marks in:

    private HashMap<Class<?>, HashMap<Entity, ? extends Component>> m_componentStores;

So it becomes legal to put an instance of some subclass of Component in this structure using some other class (not even a subclass of Component) as the key.

Remember that generic types are resolved at compile time only, so at run time m_componentStores has no way of knowing what exact type of the value you have in there, other than that it extends Component.

So the type you get from store.get(e) is ... Component:

    Component result = store.get(e);

When you cast Component to T, the compiler issues a warning because the cast cannot be checked statically. But if you're sure of the semantics of your data structure, you can simply suppress the warning.

    @SuppressWarnings("unchecked")
    T resultT = (T)result;

PS: You don't need a wildcard capture, the following will work exactly the same in your case:

    private HashMap<Class<?>, HashMap<Entity, Component>> m_componentStores;
Community
  • 1
  • 1
rustyx
  • 80,671
  • 25
  • 200
  • 267