1
\$\begingroup\$

This is a quick and dirty QBE method that covers about 60% of my use cases in my DAO. I just wanted to see if someone would take a quick look at it to see if there are any issues.

public List<Order> orderQuery(Order order) {

    StringBuilder sql = new StringBuilder();
    sql.append("select * from orders ");

    List<String> clauses = new ArrayList<String>();
    List<Object> values = new ArrayList<>();

    if (order.getOrderID() != null) {
        clauses.add("order_id = ?" + order.getOrderID());
        values.add(order.getOrderID());
    }

    if (order.getAccountCode() != null) {
        clauses.add("acct_nmbr = ?");
        values.add(order.getAccountCode());
    }

    // TODO: Once I switch this to the AccountType enum, use order.getAccountType().getCode()
    if (order.getAccountType() != null) {
        clauses.add("acct_typ = ?");
        values.add(order.getAccountType());
    }

    if (order.getRsid() != null) {
        clauses.add("rsid = ?");
        values.add(order.getRsid());
    }

    if (order.getUserID() != null) {
        clauses.add("user_id = ?");
        values.add(order.getUserID());
    }

    if (order.getCurrentStatus() != null) {
        values.add(order.getCurrentStatus());
    }

    if (!clauses.isEmpty()) {
        sql.append("where ");
    }

    String whereClause = clauses.stream().collect(Collectors.joining(" and "));
    sql.append(whereClause);

    List<Order> result = jdbcTemplate.query(sql.toString(), orderRowMapper, values.toArray());
    return result;
}

Generally it's simple: 1. Put the SQL clause in a StringBuilder. 2. Check the various fields I care about and if they aren't null, add a clause for them. 3. Turn the list of SQL where clauses into a single string 4. Build the entire SQL statement. 5. Execute it with Spring JDBC Template.

Any external input?

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

Be careful of SQL injections.

Assuming that the Order object is sanitized, then the only suggestion I'd make is to make a class for your order query and extract the logic from your method into that class.

A quick example would be that:

public class OrderQuery {

    private List<String> clauses = new ArrayList<>();
    private List<Object> values = new ArrayList<>();

    public OrderQuery(Order order) {
        setOrderId(order.getOrderID());
        // ...
    }

    private void setOrderId(String orderId) {
        if (orderId != null) {
            clauses.add("order_id = ?");
            values.add(orderId);
        }
    }

    // ...

    public List<Object> getValues() {
        return values;
    }

    @Override
    public String toString() {
        StringBuilder sql = new StringBuilder();
        sql.append("select * from orders ");
        if (!clauses.isEmpty()) {
            sql.append("where ");
        }
        String whereClause = clauses.stream().collect(Collectors.joining(" and "));
        sql.append(whereClause);
        return sql.toString()
    }
}

And to call it from your now much less clustered method:

public List<Order> orderQuery(Order order) {
    OrderQuery orderQuery = new OrderQuery(order)
    List<Order> result = jdbcTemplate.query(orderQuery.toString(), orderRowMapper, orderQuery.getValues().toArray());
    return result;
}

But it could be further optimized by using something like builder pattern.

\$\endgroup\$
  • \$\begingroup\$ Doing my best to prevent sql injections, the parameterization should help. Thanks for looking at the code. \$\endgroup\$ – Jason Nov 5 '18 at 16:33

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.