Get rid of if-else ladder when creating JPA criter

2020-07-22 09:38发布

问题:

I'm using,

  • JPA 2.0
  • Mojarra 2.1.9
  • JSF component library, Primefaces 3.5.
  • MySQL 5.6.11

I have a table in MySQL database named state_table with three columns as an example.

  • state_id (BigInt)
  • state_name (Varchar)
  • country_id (BigInt)

state_id is a auto-generated primary key and country_id is a foreign key that references a primary key of the country table.


This table is mapped by its corresponding entity class named StateTable and the data held by this table are displayed in a Primefaces DataTable, <p:dataTable>...</p:dataTable>.

The DataTable column header contains a clickable sort area, <div> for each column with a sort direction for sorting, when this area is clicked, a String, either ASCENDING or DESCENDING representing the sort order is rendered and a text box for filtering (searching) in which a user enters a search item for each column.


So ultimately, what I get in JSF managed bean is a List of type java.util.List<org.primefaces.model.SortMeta> representing sort orders of the columns of the DataTable that a user wishes.

And a Map of type java.util.Map<java.lang.String, java.lang.String> representing the search column names as keys and search items of the corresponding columns as values (a search item is entered by a user in a text box on the column header of each column of DataTable).


In short, I use List<SortMeta> for sorting and Map<String, String> for filtering/searching.

My code in one of the DAOs to get a list of rows after sorting and filtering is as follows.

@Override
@SuppressWarnings("unchecked")
public List<StateTable> getList(int first, int pageSize, List<SortMeta> multiSortMeta, Map<String, String>filters)
{
    CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
    CriteriaQuery<StateTable> criteriaQuery = criteriaBuilder.createQuery(StateTable.class);
    Metamodel metamodel=entityManager.getMetamodel();
    EntityType<StateTable> entityType = metamodel.entity(StateTable.class);
    Root<StateTable>root=criteriaQuery.from(entityType);
    Join<StateTable, Country> join = null;

    //Sorting

    List<Order> orders=new ArrayList<Order>();

    if(multiSortMeta!=null&&!multiSortMeta.isEmpty())
    {
        for(SortMeta sortMeta:multiSortMeta)
        {
            if(sortMeta.getSortField().equalsIgnoreCase("stateId"))
            {
                orders.add(sortMeta.getSortOrder().equals(SortOrder.ASCENDING)?criteriaBuilder.asc(root.get(StateTable_.stateId)):criteriaBuilder.desc(root.get(StateTable_.stateId)));
            }
            else if(sortMeta.getSortField().equalsIgnoreCase("stateName"))
            {
                orders.add(sortMeta.getSortOrder().equals(SortOrder.ASCENDING)?criteriaBuilder.asc(root.get(StateTable_.stateName)):criteriaBuilder.desc(root.get(StateTable_.stateName)));
            }
            else if(sortMeta.getSortField().equalsIgnoreCase("country.countryName")) // Yes, Primefaces DataTable renders this ugly name in case of a nested property representing a foreign key relationship.
            {
                join = root.join(StateTable_.countryId, JoinType.INNER);
                orders.add(sortMeta.getSortOrder().equals(SortOrder.ASCENDING)?criteriaBuilder.asc(join.get(Country_.countryName)):criteriaBuilder.desc(join.get(Country_.countryName)));
            }
        }
    }

    //Filtering/searching

    List<Predicate>predicates=new ArrayList<Predicate>();

    if(filters!=null&&!filters.isEmpty())
    {
        for(Entry<String, String>entry:filters.entrySet())
        {
            if(entry.getKey().equalsIgnoreCase("stateId"))
            {
                predicates.add(criteriaBuilder.equal(root.get(StateTable_.stateId), Long.parseLong(entry.getValue())));
            }
            else if(entry.getKey().equalsIgnoreCase("stateName"))
            {
                predicates.add(criteriaBuilder.like(root.get(StateTable_.stateName), "%"+entry.getValue()+"%"));
            }
            else if(entry.getKey().equalsIgnoreCase("country.countryName"))// Yes, Primefaces DataTable renders this ugly name in case of a nested property representing a foreign key relationship.
            {
                if(join==null)
                {
                    join = root.join(StateTable_.countryId, JoinType.INNER);
                }
                predicates.add(criteriaBuilder.like(join.get(Country_.countryName), "%"+entry.getValue()+"%"));
            }
        }
    }

    if(predicates!=null&&!predicates.isEmpty())
    {
        criteriaQuery.where(predicates.toArray(new Predicate[0]));
    }

    if(orders!=null&&!orders.isEmpty())
    {
        criteriaQuery.orderBy(orders);
    }
    else
    {
        criteriaQuery.orderBy(criteriaBuilder.desc(root.get(StateTable_.stateId)));
    }
    TypedQuery<StateTable> typedQuery = entityManager.createQuery(criteriaQuery).setFirstResult(first).setMaxResults(pageSize);
    return typedQuery.getResultList();        
}

This works as expected but as it can be noticed, the if-else if ladder inside the foreach loop can contain many conditional checks as the number of columns in a database table are increased.

Each column requires a conditional check for both sorting and searching. Is there an efficient way to get rid of these conditional checks that can ultimately remove or at least minimize this if-else if ladder?

P.S. In case of country, I'm doing sorting and searching on countryName (which is available in the parent table country) rather than countryId. Hence, I'm using Join, in this case.

回答1:

If you drop the usage of SingularAttribute values and you make sure that the caller calls the method with exactly the desired column names in sort/filter fields, then you could simplify it a lot more by just reusing the iterated sort/filter field as column name without the need for an if/else check on the field in order to specify the right column name (which is after all actually identical to the sort/filter field name).

Essentially, you don't need those equalsIgnoreCase() checks in if-else ladder at all. As to case sensitivity, if the caller is doing it wrong, just fix it over there instead of being too forgiving on caller's mistakes.

Here's how you could refactor it then:

/**
 * @throws NullPointerException When <code>multiSortMeta</code> or <code>filters</code> argument is null.
 */
@SuppressWarnings({ "unchecked", "rawtypes" })
public List<?> getList(int first, int pageSize, List<SortMeta> multiSortMeta, Map<String, String> filters) {
    // ...

    Root<StateTable> root = criteriaQuery.from(entityType);
    Join<StateTable, Country> join = root.join(StateTable_.countryId, JoinType.INNER);

    List<Order> orders = new ArrayList<Order>();

    for (SortMeta sortMeta : multiSortMeta) {
        String[] sortField = sortMeta.getSortField().split("\\.", 2);
        Path<Object> path = sortField.length == 1 ? root.get(sortField[0]) : join.get(sortField[1]);
        orders.add(sortMeta.getSortOrder() == SortOrder.ASCENDING 
            ? criteriaBuilder.asc(path) 
            : criteriaBuilder.desc(path));
    }

    List<Predicate>predicates = new ArrayList<Predicate>();

    for (Entry<String, String> filter : filters.entrySet()) {
        String[] filterField = filter.getKey().split("\\.", 2);
        Path path = filterField.length == 1 ? root.get(filterField[0]): join.get(filterField[1]);
        predicates.add(filter.getValue().matches("[0-9]+") 
            ? criteriaBuilder.equal(path, Long.valueOf(filter.getValue()))
            : criteriaBuilder.like(path, "%" + filter.getValue() + "%"));
    }

    // ...
}

Note that I also modified the method to not accept null as sort and filter meta, so that you can safely trim out all those null checks. Those empty checks are unnecessary as the for loop won't iterate anyway if it's empty. Also note that the filtering uses CriteriaBuilder#equal() if a numeric input is given, otherwise it uses like(). I'm not sure if that covers all your cases, you may want to finetune that more.

You can if necessary refactor the obtaining of Path even more with the following helper method:

@SuppressWarnings("rawtypes")
private static Path<?> getPath(String field, Root root, Join join) {
    String[] fields = field.split("\\.", 2);
    return fields.length == 1 ? root.get(fields[0]): join.get(fields[1]);
}