Do CriteriaQuery code more readable and elegant

Question

I wrote the code and would like to change it to more readable and elegant:

@Repository
public class ProductRepositoryImpl implements ProductCustomRepository {

private static final int MORE_THEN_RESULT = 1;

@PersistenceContext
private EntityManager em;

public List<Product> findByFilter(Filter filter) {
    final CriteriaBuilder cb = em.getCriteriaBuilder();
    final CriteriaQuery<Product> cq = cb.createQuery(Product.class);
    final Root<Product> from = cq.from(Product.class);

    try {
        final List<Predicate> predicates = createPredicatesByFilter(cb, from, filter);
        if (!CollectionUtils.isEmpty(predicates)) {
            cq.where(cb.and(predicates.toArray(new Predicate[predicates.size()])));
        }
    } catch (RuntimeException e) {
        return Collections.emptyList();
    }

    cq.select(from);

    return em.createQuery(cq).getResultList();
}

private List<Predicate> createPredicatesByFilter(CriteriaBuilder cb, Root<Product> from, Filter filter) {
    final List<Predicate> predicates = new ArrayList<>();

    if (!StringUtils.isEmpty(filter.getName())) {
        predicates.add(cb.like(cb.lower(from.<String>get("name")), filter.getName().toLowerCase() + "%"));
    }

    final BigDecimal priceBefore = filter.getPriceBefore();
    final BigDecimal priceAfter = filter.getPriceAfter();

    if (priceBefore != null && priceAfter != null) {
        if (priceBefore.compareTo(priceAfter) != MORE_THEN_RESULT) {
            predicates.add(cb.between(from.<BigDecimal>get("price"), priceBefore, priceAfter));
        } else {
            throw new RuntimeException();
        }
    } else if (priceBefore != null) {
        predicates.add(cb.gt(from.<BigDecimal>get("price"), priceBefore));
    } else if (priceAfter != null) {
        predicates.add(cb.lt(from.<BigDecimal>get("price"), priceAfter));
    }

    if (!StringUtils.isEmpty(filter.getCategory())) {
        predicates.add(cb.like(cb.lower(from.<Category>get("category").<String>get("name")), filter.getCategory().toLowerCase() + "%"));
    }

    return predicates;
}
}

Answer 1

In general this looks okay to me. In fact I have a similar pattern in an application with lots of optional filters. Newer Java 8 features which can make this a bit nicer (e.g. using Optional and method references).

• Throwing/catching RuntimeException like that looks iffy. At least add a message to the exception so the reader knows how to interpret this.
• Returning a List only to immediately convert it to an array doesn't sound that useful, then again, if it's being used from other places it might be okay. List also has isEmpty, better than having to call yet another static function for it.

• CriteriaQuery.where is already using the conjunction, no need to add the and there. Note that the array can be passed for the varargs arguments:

  if (!CollectionUtils.isEmpty(predicates)) {
      cq.where(predicates.toArray(new Predicate[predicates.size()]));
  }
  

• For the the string matching queries additional helper methods might be nice:

  private static startsWithLower(Expression<String> expression, String pattern) {
      return cb.like(cb.lower(expression), pattern.toLowerCase() + "%");
  }