What's the best way to deal with potential runtime exceptions from a java “unchecked conversion”?

Question

So I have a function that looks like this:

@SuppressWarnings("unchecked")
public static <E> Set<E> getSetOfClass(Query q,Class<E> clazz) {
    return new LinkedHashSet<E>( q.getResultList() );
}

What I believe this is doing is taking a javax.persistence.Query and returning its result set as a generic Set<Class>.

This seems like a great solution, but first off, is it actually doing what I think it is, and is this the best way to achieve that? I find it odd that I never reference my clazz param, but it does seem to do what I want it to.

Secondly if this is all correct and sensible, what errors can this throw? I'd imagine if I give it an incorrect class this function wouldn't work, though I'm not sure at all on that.

If I do something like:

Query q = em.createQuery("FROM Element");
Set<Fish> s = MyUtil.getSetOfClass( q, Fish.class);

And if Fish is not a superclass of Element then what happens? Should I assume that this function will always be used correctly or should I be doing so error handling? What are people recommendations of a best practice approach?

Regards,

Glen x

Answer 1

The getSetOfClass does not guarantee that all elements in the Set are objects of type E. If you call it incorrectly (what you always can), like:

 Set<Cat> cats = getSetOfClass(findAllFish(), Cat.class);

you'll receive class cast exceptions at various places later on...

I'd add some checks to the public getSetOfClass method to guarantee that the content of the set matches the type parameter:

@SuppressWarnings("unchecked")
public static <E> Set<E> getSetOfClass(Query q,Class<E> clazz) {
   return getSetOfClass(Query q,Class<E> clazz, true);
}

@SuppressWarnings("unchecked")
public static <E> Set<E> getSetOfClass(Query q,Class<E> clazz, boolean validate) {
    List result = q.getResultList();
    if (validate) {
       for (Object o:result) {
          if(!o.getClass().equals(clazz)) {
              // handle error
          }
       }
    }
    return new LinkedHashSet<E>( result );
}

Answer 2

To add to @Andreas_D's answer, just remember that all Java generics information is only used to check your code for type correctness at compile time, and is erased at run-time. Therefore, effectively what you get is something like this:

public static Set<Object> getSetOfClass(Query q,Class<Object> clazz) {
  return new LinkedHashSet<Object>( q.getResultList() );
}

Which means at runtime everything will just work, as far as the above method goes.
Update: As kindly pointed out by @Blaisorblade, the getSetOfClass method could use the clazz to check for type correctness and fail-fast if the type is wrong. Although it cannot be done at compile time, it'd make it easier to pin-point the problem in case of failure at runtime.

Now assuming that later you have:

Query q = em.createQuery("FROM Element");
Set<Fish> s = MyUtil.getSetOfClass( q, Fish.class);
for(Fish fish : s){
  fish.swim();
}

Then at runtime it will look like:

Query q = em.createQuery("FROM Element");
Set<Object> s = MyUtil.getSetOfClass( q, Fish.class);
for(Object fish : s){
  ((Fish)fish).swim();
}

Now you can see what will happen if elements are of type Cat. The (Fish)fish part will throw a ClassCastException (if it gets that far).

Generics are therefore really useful when the type information can be tracked by the compiler without any unchecked warnings from beginning the the end. For other cases (like yours) where the generics is "hacked" in the middle, it cannot guarantee the correctness of you program. This is inevitable, especially in cases where the data is persisted to disk or a database, since there is no way to be sure that the persisted data is of correct type. The programmer has to just be careful.

Answer 3

I've never been sure whether I like generics or not. In this case it seems they would be a good idea and would have saved you a lot of trouble. As Persistence does not (yet) seem to support them, I'll put on my anti-generics hat and explain how real Java programming is done.

My suggestion, then, would be to give up on the generics and classes and just return a plain old Set:

public static Set getSetOfClass( Query q )  {
    return new LinkedHashSet( q.getResultList() );
}

(use @SuppressWarnings("unchecked") as needed, if you can't get a 1.4 compiler.)

If the query contains nothing but E's, you'll never have a problem. Or at any rate, the trouble will be rare, obvious at run time, and best dealt with by the programmers who are misusing your method. Nothing says "Change your basic approach" like an unexpected ClassCastException at run time.

If there are the occasional, legitimate, non-E Fish objects in there, the programmers who are using the method are better placed to deal with them than you are. They can check at run time and optionally throw out the individual fish or toss the whole Set as suits their purpose, which they know and you do not.

If you do know their purposes, then you can perhaps save them some trouble by adding in the Clazz parameter so you know what they want. Then you can do the filtering or return a null or throw your own checked exception or return a special class object that explains in vast detail the nature of the Set contents. But be sure you don't do more work than you are saving the method's users.