codesmell.org

Don’t get me wrong: i like annotations, i like JPA etc etc… But there are some things, that – at least – i don’t really get. One thing is using Named Queries realized with annotations, and here is why:

As you already know, named queries are a good thing. Compared to the dynamic query creation approach, they provide a good set of advantages:

1. They force the developers to organize their Queries and define them together in a ‘more or less’ common place.
2. They prevent SQL-injection approaches by forcing the developer to use parameters that are correctly escaped by the query engine automatically.
3. They therefore make it possible for the query engine to translate EJBQL/JPQL to the target SQL Dialect only once, and the reuse the query with different parameters.

So in theory, i am a fan of named queries. This changed a little while actually using them.

So here is what you do in EJB3 to define and use named queries. We start by referencing a named query, which looks straightforward:

 Java | 
 copy |
?
 
@Stateless public class QueryTest{ 
  @PersistenceContext(unitName= "..." ) 
  EntityManager manager; 
  public Collection list(String color){ 
    return (Collection) manager.createNamedQuery( "findAllByColor" ).setParameter( "Color" ,color).getResultList(); 
  } 
}

but when defining them, things get clumsy:

 Java | 
 copy |
?
 
@NamedQueries({ 
  @NamedQuery(name= "findAll" ,query= "SELECT c FROM Cars c" ), 
  @NamedQuery(name= "findAllByColor" ,query= "SELECT c FROM Cars c WHERE Color=:Color" ) })
public class Car { // ... }

First thing is the ‘NamedQueries’ annotation. This is much more XML-like than i like it. From an application developer’s view: why the hell can’t we just use zero or more NamedQuery annotations? Yes. i know the technical reasons, but this solution is a little clumsy.

More important: The names are unfortunately global. This leads to the obvious solution to prefix them with the Entity Name: “Car.findAll”.
Now imagine a situation, where your query returns a primitive value and is not unambiguously bound to one table/entity. Two Problems arise: “In which class to define it and how to prefix it ?”

Obviously you’ll want to keep your naming scheme consistent, so you’ll at least prefix it with the name of the class it is defined in, so that your coworker is going to find it. Sounds familiar?
Talking about your coworker: how does he know which name you used when defining the queries? He obviously needs to look at the source (this might change when tool support becomes better) and be careful not to mistype the name, because that’s going to lead to a runtime error. Given he managed that, his code will break when you start refactoring.

This scenario does not really sound that new. We’ve fought this battle a hundred times. The simple and obvious solution is: To hell with named Queries and use what you’ve always used: constants.

 Java | 
 copy |
?
 
@Stateless public class QueryTest{ 
  @PersistenceContext(unitName= "..." ) 
  EntityManager manager; 
  public Collection list(String color){ 
    return (Collection) manager.createQuery(Car.QUERY_findAllByColor).setParameter( "Color" ,color).getResultList(); 
  } 
}

using:

 Java | 
 copy |
?
 
public class Car { 
  public static final String QUERY_findAll= "SELECT c FROM Cars c" ; 
  public static final String QUERY_findAllByColor= "SELECT c FROM Cars c WHERE Color=:Color" ; // ... 
}

Several advantages come to mind instantly:

1. Your coworker won’t mistype the name given that he uses a decent IDE with code-completion
2. Even if he does mistype, it’ll lead to a compiler-error instead of runtime
3. The prefixing is done by the compiler and therefore consistent
4. Prefixing does not have to be touched when refactoring
5. You can control the scope of the Query (i.e. make it package-local or something)

While gaining all this you still enjoy the advantages of secure and reusable queries, given the query engine is worth its name.

Considering XML-defined named queries to be much more harmful and clumsy than annotation-based definition, i’ll stick to constants. Are you with me?