Code Review Asked by stefan.georgiev.uzunov on September 23, 2020
I’ve created a little class which helps me to generalize many query cases. I have a DbService and couple methods in it. Can you please take a look at it and share some criticism? The usage of that tool is really easy to me and I do wanna know how to make it safer and optimized.
public class DbServiceImpl implements DbService {
private final EntityManager entityManager;
@Inject
DbServiceImpl(EntityManager entityManager) {
this.entityManager = entityManager;
}
@Override
public <T, V> List<V> createQuery(Query<T, V> query) {
try {
if (query != null && query.entityType.getAnnotation(Entity.class) != null) {
CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
CriteriaQuery<V> criteriaQuery = criteriaBuilder.createQuery(query.returnType);
query.root = criteriaQuery.from(query.entityType);
query.builder = criteriaBuilder;
Selection<? extends V> selection = query.select(); //invokes on the call.
Predicate predicate = query.where();
if (predicate != null) {
criteriaQuery.where(predicate); //null of a simple select without where clause.
}
criteriaQuery.select(selection); //null is allowed, it will just select * from the entity
return entityManager.createQuery(criteriaQuery).getResultList();
}
} catch (Exception exception) {
exception.printStackTrace();
}
return new ArrayList<>(); //a simple check of .isEmpty() covers that case.
}
@SuppressWarnings("unchecked")
public abstract static class Query<T, V> {
private final Class<T> entityType;
private final Class<V> returnType;
//accessible only by outer class's methods.
private Root<T> root;
private CriteriaBuilder builder;
public Query() {
this.entityType=(Class<T>) ((ParameterizedType) getClass()
.getGenericSuperclass()).getActualTypeArguments()[0];
this.returnType=(Class<V>) ((ParameterizedType) getClass()
.getGenericSuperclass()).getActualTypeArguments()[0];
}
protected abstract Selection<? extends V> select();
protected abstract Predicate where();
protected Root<T> root() {
return root;
}
protected CriteriaBuilder builder() {
return builder;
}
}
}
And here are some use cases. Passed types are as follows – entity & return (User,Long)
List<Long> data = dbService.createQuery(new DbServiceImpl.Query<User, Long>() {
@Override
protected Selection<? extends Long> select() {
return builder().count(root());
}
@Override
protected Predicate where() {
return root();
}
});
This allows me to find the count of a table so understandable, without a line of pure HQL or any SQL code. One more example:
List<User> users = dbService.createQuery(new DbServiceImpl.Query<User, User>() {
@Override
protected Selection<? extends User> select() {
return root();
}
@Override
protected Predicate where() {
return builder().equal(root().get("username"), username);
}
});
One with a join:
List<Recipe> recipeList= dbService.createQuery(new DbServiceImpl.Query<Recipe, Recipe>() {
@Override
protected Selection<? extends Recipe> select() {
return null;
}
@Override
protected Predicate where() {
root().fetch("author", JoinType.LEFT);
return builder().equal(root().get("author").get("id"),user.getId());
}
});
The code structure remains the same but the query is different. The access modifiers I used allowed me to encapsulate the behaviour on the use cases. What do you think? How broken or useful is that?
Welcome to Code Review, I have seen your code and I have seen you encapsulated your createQuery
method like below:
try { body method } catch (Exception exception) { exception.printStackTrace(); }
You are basically catching all NullPointerException
cases and other runtime exceptions you can meet within your method with the Exception
class, this is a sign something should be redesigned to avoid these situations.
About the paradigm you are following , you are implementing a personal version of the repository pattern, referring to structures like the jpa repository present for example in the spring framework. You can check while you are creating different queries worrying about the construction of a valid query (and this is the reason why you had to encapsulate your method in the try catch
construct), it should be better using an already built class to do the same things with much less effort.
Answered by dariosicily on September 23, 2020
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP