Inderjeet Singh's Blog

Improving support for generics in the Java Persistence API

The Java Persistence API comes in handy for creating object relational mapping. I recently came across a warnings that the compiler generates on some code that uses these APIs in our Web 2.0 Pet Store project. Upon a closer look, I concluded that the warning was bogus and came up with a suggestion for the Persistence API expert group to better support generics. Here is the warning in question:

D:\ws\petstore\ws\apps\petstore\src\java\com\sun\javaee\blueprints\petstore\model\CatalogFacade.java:47: warning: [unchecked] unchecked conversion
found : java.util.List
required: java.util.List<com.sun.javaee.blueprints.petstore.model.Category>


Here is the code that causes this warning to be generated:
Query query = em.createQuery("SELECT c FROM Category c");
List<Category> categories = query.getResultList();

Essentially, the compiler is complaining that query.getResultList() returns just List whereas we are expecting it to be List<Categories>. This warning is unfortunate because we know that the query results are going to be list of Category objects, and we would like to express it somehow. Ideally, javax.persistence.Query should allow us to express this by providing an additional method:
List<T> getResultList(Class<T> c);

This will enable us to write the following code:
List<Category> categories = query.getResultList(Category.class);
and the warning will get eliminated and we will have improved type-safety.

I hope the Java Persistence expert group will look at this issue and add a generic version to ensure better type-safety. I hope they will look at other API methods as well to see if better support for generics can be added.

What is your opinion on this matter? Do you have other suggestions for improving the Java Persistence API? Share your thoughts as comments to this blog. Thanks for reading.

Comments

• I'm not a Persistence Guru, but I would think it would be better to specify the type at createQuery() time and not getResultList(). It just feels too much like a cast. I know you can't parse the string, so would it be possible to parametrize the create query call?

  
  Query query = em.createQuery("SELECT c FROM %1$ c", Category.class);

  List categories = query.getResultList();

  The "%1$" would be replaces by the class name, and still have the type around so that getResultList() would return the correct type. I've used the java Formatter syntax, but there might be a better one.

  Posted by: aberrant on January 03, 2007 at 03:33 PM

• Hi aberrant, I agree with your comment. It will be better for the createQuery method to take in the class parameter.

  Posted by: inder on January 03, 2007 at 03:40 PM

• Isn't it enough to make the Query interface generic?

  interface Query<E> {
  List<E> getResultList();
  E getSingleResult();
  ...
  }

  Of course the various EntityManager.createQuery methods need also be made generic:

  <E> Query<E> createQuery(String string);
  ...

  Posted by: odd on January 04, 2007 at 05:21 AM

• In the grammar point of view, it should make the method generic; but in the logic point of view, Query already has got the class information from the SQL (It already created Category list, isn't it/). Adding a redundant Class in this method just for grammar reason I think it's not a good idea. It's better the compiler can check this from the SQL or Java can implement dynamic generics in the future.

  Posted by: zwe on January 04, 2007 at 05:39 PM

• All these suggestions can't avoid the wrong use cases, just no compile errors. for example:

  
  Query q = em.createQuery("SELECT f FROM Foo f");
  List<Bar> rs = q.getResultList(Bar.class);

  Query<Bar> q = em.createQuery("SELECT f FROM Foo f");
  List<Bar> rs = q.getResultList();

  
  What's the difference with the current one?

  Posted by: zwe on January 04, 2007 at 05:52 PM

• Hi odd, I agree with your suggestion as well. Any of these options will work, but it will be best to have the Query interface itself make everything typesafe. (You do have a minor typo in there with an extra return value ).

  Posted by: inder on January 05, 2007 at 02:27 PM

• Hi zwe, I dont fully understand your concern, but will try to answer it anyway. The problem with the existing situation is that the compiler generates a warning because the result needs to be type-casted to List but List has no meaning at runtime since generic type information is not maintained beyond compilation.

  Posted by: inder on January 05, 2007 at 02:30 PM

• I mean void the compile waring has cost - the suggested new API is not so clear and simple as the current one - We have told Query that the list is Category by EBJQL. The generated item of the result list is type Category and not just a simple Java Object indeed. What is the benefit of new suggested API? There is no compile warning ONLY. To let me choose from 1) Compile warning + simple API and 2) NO compile waring + API with redundant parameter I would rather choose 1).

  Posted by: zwe on January 07, 2007 at 09:15 PM

• And also, using a wrong class to call your suggest API can't be detected at compile time either, eg:

  Query query = em.createQuery("SELECT c FROM Category c");
  List categories = query.getResultList(Product.class);

  
  

  Posted by: zwe on January 07, 2007 at 09:20 PM

• Hi aberrant and odd: I created prototypes for the various classes to verify if it is possible to parameterized the Query class.
  public class ParameterizedQuery<E> {
    public Collection<E> getResultList() {
      return new ArrayList<E>();
    }
  }
  public class EntityManager {
    public <T> ParameterizedQuery<T> createQuery(Class<T> c, String s) {
      return new ParameterizedQuery<T>();
    }
  ...
  }

  This is invoked as:

  ParameterizedQuery gq = em.createQuery(Category.class, query);
  result = gq.getResultList();

  However, this still results in the same unchecked conversion warning.

  Seems like the only good way to get rid of the warning is to use the original approach that I suggested. So, the method getResultList() in the query class should be rewritten as:

  public class Query {
    public <T> Collection<T> getResultList(Class<T> c) {
      return new ArrayList<T>();
    }
  }

  
  

  Posted by: inder on January 09, 2007 at 02:07 PM

• Hi zwe, it is correct that a user can pass a different class than what the query results generate, and the compilation will not detect that error. However, it is a matter of signaling intentions. By indicating that I expect the results to be Category.class, I am making quite a strong statement that benefits anyone who reviews my code. It is also a good hint for the implementation of getResultList() which can try to do an intelligent conversion of the results to the indicated type. So, I think it is worth the extra little complexity.

  Posted by: inder on January 09, 2007 at 03:18 PM

• Part of the problem here is the relationship between
  Collection<Category> and List<Category> - there is none!

  Here's a set of classes that at least compile without warnings:

  
  import java.util.*;

  public class t {
  public static void main() {
  EntityManager em = new EntityManager();
  String query = "";
  ParameterizedQuery<Category> gq = em.createQuery(Category.class, query);
  List<Category> result = gq.getResultList();
  }
  }

  class Category {
  }

  class ParameterizedQuery<E> {
  public List<E> getResultList() {
  return new ArrayList<E>();
  }
  }

  class EntityManager {
  public <T> ParameterizedQuery<T> createQuery(Class<T> c, String s)
  {
  return new ParameterizedQuery<T>();
  }
  }

  Posted by: shannon on January 26, 2007 at 03:15 PM

• Hi Bill (Shannon), Thanks for pointing out the bug in my code and posting the revised code. You are right, Collection<E> has no relation to List<E> since subtyping does not extends to generics.

  Posted by: inder on January 26, 2007 at 03:21 PM

• List<E> is a subtype of Collection<E> (just like ArrayList<E> is a subtype of List<E>)-- I suspect that you're confusing this with the fact that List<B> is not a subtype of List<A> even if B extends A. The cause of the warning with the January 9 code is presumably because the type of the variable gq is the raw type ParameterizedQuery instead of ParameterizedQuery<Category>.

  Posted by: peterjones on April 29, 2007 at 09:03 AM

• Hi zwe, it is correct that a user can pass a different class than what the query results generate, and the compilation will not detect that error. However, it is a matter of signaling intentions. By indicating that I expect the results to be Category.class, I am making quite a strong statement that benefits anyone who reviews my code. It is also a good hint for the implementation of getResultList() which can try to do an intelligent conversion of the results to the indicated type. So, I think it is worth the extra little complexity.
  

  Posted by: ikermollas on June 12, 2007 at 12:12 AM

This work is licensed under a Creative Commons License.