How to abstract away JDBC code when I need to populate from ResultSet before closing ResultSet?

Question

I'm working on a Java application that conducts numerous JDBC queries to a database, each specified in a separate method with roughly the following format.

public static void sampleJDBCQuery(String query, DBUtil dbUtil, DataStructure dataStructure) {
    ResultSet rs = null;
    Handle handle = null;
    Connection conn = null;
    Statement stmt = null;
    LOGGER.debug("Executing query = {}", query);
    try {
        handle = dbUtil.getConnectionHandle();
        conn = handle.getConnection();
        if (conn != null) {
            stmt = conn.createStatement();
            if (stmt != null) {
                rs = stmt.executeQuery(query);
                if (rs != null) {
                    while (rs.next()) {
                        // populate dataStructure using rs
                    }
                }
            }
        }
    } catch (Exception e) {
        Metrics.markMeter("vertica.read.error");
        LOGGER.error(e.getMessage(), e);
    } finally {
        DBUtil.closeResultSet(rs);
        DBUtil.closeStatement(stmt);
        DBUtil.close(handle);
        LOGGER.debug("Finished query = {}", query);
    }
}

With many different sample queries (and data structure classes) of the format above, my code base has been growing substantially. My aim is to have a helper method that abstracts away most of this JDBC logic for me. My first thought was to have a method with the following signature.

public static ResultSet executeJDBCQuery(String query, DBUtil dbUtil)

And then I could loop over the rows of the ResultSet and populate the relevant DataStructure for each row. The problem is that I would still have to close the returned ResultSet and closing ResultSet in a different method seems like bad design.

I guess what I'm looking for is maybe something similar to Python's concept of function decorators so that I could "decorate" a JDBC query to take care of most of the boilerplate present in sampleJDBCQuery above. How could I achieve this?

Maybe what you need is some kind of delegate pattern, where you define a generic interface with a method that takes a ResultSet as a parameter an extracts the current values from it (I’d make it so the intention was that the delegate should only extract the “current” row, the responsibility for integrating through the rows remains the calling method’s) — MadProgrammer, Jan 10 '19 at 20:35
All those null checks you do should be entirely unnecessary; those methods should never return `null` (except maybe that `handle.getConnection`, but if it can return `null`, than you should fix that). — Mark Rotteveel, Jan 11 '19 at 13:12
If there is no requirement that forces to use JDBC go with a ORM like Hibernate before re-inventing the wheel . — PeterMmm, Jan 11 '19 at 17:29
Hibernate is great in some ways. But it has a learning curve and can make things more obscure. sure, check it out, but it's not for everybody. — Nathan Hughes, Jan 11 '19 at 17:31

Nathan Hughes · Accepted Answer · 2019-01-14T17:01:51.663

You can pass in a strategy to tell the method how to map the row to fields on an object.

This is what spring-jdbc does, it defines a RowMapper as:

public interface RowMapper<T> {
    T mapRow(ResultSet resultSet, int rowNum) throws SQLException;
}

This is how you could change your method, including incorporating the rowMapper:

public static <T> List<T> queryList(String query, Connection conn, RowMapper<T> rowMapper) throws SQLException {
    LOGGER.debug("Executing query = {}", query);
    try {
        Statement stmt = conn.createStatement();
        ResultSet rs = stmt.executeQuery(query);
        List<> list = new ArrayList<>();    
        while (rs.next()) {
            list.add(rowMapper.mapRow(rs, list.size() + 1)); 
        }
        return list;
    } finally {
        DBUtil.closeResultSet(rs);
        DBUtil.closeStatement(stmt);
        LOGGER.debug("Finished query = {}", query);
    }
}

Catching all exceptions here isn't good because you want to be able to use the exception to bail out of the current operation if something is going wrong. Otherwise you will have multiple error stacktraces in the log, one here where the error happens, then another downstream where your code fails when it expects a result to be present that isn't due to the previous error. Just fail fast when the first error happens.

The next step is going to be parameterizing your queries so you don't have to surround parameter values in quotes or worry about sql injection. See this answer for an example of how spring-jdbc handles this.

I moved the connection stuff out of the method; passing in a connection allows you to have multiple sql statements execute in the same JDBC local transaction. (The connection still needs to get closed, this is just the wrong place to do it.)

Also passing in this handle thing here is a violation of the Law of Demeter:

In particular, an object should avoid invoking methods of a member object returned by another method. For many modern object oriented languages that use a dot as field identifier, the law can be stated simply as "use only one dot". That is, the code a.b.Method() breaks the law where a.Method() does not. As an analogy, when one wants a dog to walk, one does not command the dog's legs to walk directly; instead one commands the dog which then commands its own legs.

Even if you're only doing read-only stuff, having queries share a transaction can improve consistency and be better for performance. Using transactions is less painful with Spring, you can implement transactions declaratively using annotations to show where you want the boundaries to be.

The big-picture answer here is: it would be better to adopt a pre-existing tool (spring-jdbc or something similar), where they have already solved problems you haven't even gotten around to considering yet, than to reinvent it piecemeal, which is the road you're evidently headed down.

How to abstract away JDBC code when I need to populate from ResultSet before closing ResultSet?

1 Answers1