3
\$\begingroup\$

I'm using JPA 2.0, Mojarra 2.1.9 and its component library Primefaces 3.5. I have a table in MySQL database named state_table with three columns.

  • 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.

\$\endgroup\$
  • \$\begingroup\$ You probably have to use introspection (or Commons BeanUtils), to get rid of the if-else-if's. Something like: "if the key does not contain a dot, get the property with this name from the main table; if it contains a dot, get the property from the joined table". This will create a constant-size algorithm for any number of properties. \$\endgroup\$ – Viliam Búr Aug 8 '13 at 14:39
3
\$\begingroup\$

I believe, you should not mix your architecture layers. SortMeta is a GUI class. If you want to change your GUI-Framework, or use another one, you have to refactor your Finder. Therefore you should not use SortMeta (GUI-Class) in your business layer.

\$\endgroup\$
1
\$\begingroup\$

I suggest you to wrap parameters

int first, int pageSize, List<SortMeta> multiSortMeta, Map<String, String>filters

And don't mix classes from primefeaces and dao as suggested in axels comment. When creating this wrapper convert this view classes to some custom backend classes.

in some object. I have DataTableProperites class. It is much readible and convenient to use. Instead of multiSortMeta!=null&&!multiSortMeta.isEmpty() use CollectionUtils.isEmpty

I recommend you to extract nested ifs and loops to methods. Maybe when you extract them you will find some place where you can remove duplication.

\$\endgroup\$
  • \$\begingroup\$ Regarding CollectionUtils.isEmpty: or you can just leave your null check as is and remove !multiSortMeta.isEmpty() that doesn't do anything anyway... And leave the "commons" dependency for what it is (something not that common). \$\endgroup\$ – ymajoros Aug 22 '14 at 20:08
1
\$\begingroup\$

This part of your code is not very readable:

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)));
}

What I would do is to store several things in local variables, so you don't have to repeat calls to getters over and over again. Since your call to orders.add is performed in all if-statements, it can be placed after the ifs to reduce code duplication.

String sortField = sortMeta.getSortField();
YourType sortOrder = sortMeta.getSortOrder();
YourType id = null;
if(sortField.equalsIgnoreCase("stateId") || )
{
    id = root.get(StateTable_.stateId);
}
else if(sortField.equalsIgnoreCase("stateName"))
{
    id = root.get(StateTable_.stateName);
}
else if(sortField.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);
    id = join.get(Country_.countryName);
}

if (id != null)
    orders.add(sortOrder.equals(SortOrder.ASCENDING) ? criteriaBuilder.asc(id) : criteriaBuilder.desc(id));
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.