Java database query logic

Question

I'm making a wrapper for querying the database so that I can more easily control the flow of database management and to make it easier for me to write data beans around my database. This code is going to be used a ton so I'm looking to make it as clean as possible.

/**
 * Query the database.
 * 
 * @param sql
 *            The query statement
 * @return List of rows returned by the query.
 */
public List<Map<String, Object>> query(String sql) {
    return this.query(sql, null);
}

/**
 * Query the database.
 * 
 * @param sql
 *            The query statement
 * @param params
 *            The prepared statement parameters (in order).
 * @return List of rows returned by the query.
 */
public List<Map<String, Object>> query(String sql, Object[] params) {
    final String METHODNAME = "query";
    List<Map<String, Object>> resList = null;

    try {
        if (conn != null && !conn.isClosed()) {
            resList = new ArrayList<Map<String, Object>>();

            PreparedStatement stmt = conn.prepareStatement(sql);
            if (params != null && params.length > 0) {
                for (int i = 1; i <= params.length; i++) { // 1 based, not 0
                    stmt.setObject(i, params[i - 1]);
                }
            }

            ResultSet res = stmt.executeQuery();

            while (res.next()) {
                Map<String, Object> row = new HashMap<String, Object>();

                for (int i = 1; i <= res.getMetaData().getColumnCount(); i++) {
                    String key = res.getMetaData().getColumnName(i);
                    Object val = res.getObject(i);
                    row.put(key, val);
                    resList.add(row);
                }
            }

        } else {
            logger.log(LogLevel.ALL, CLASSNAME, METHODNAME, "Attempted to run query when "
                    + "there was no established connection to the database.");
        }
    } catch (Throwable e) {
        logger.log(LogLevel.ERROR, CLASSNAME, METHODNAME,
                "An error occurred when trying to run the query.");
    }

    return resList;
}

Answer 1

Resource-leak: you neither close the statement nor the result set. Depending on the database driver, this can lead to system resource leaks, blocked OS handles, etc. Always close database objects ASAP (apart from connections, which you can hold for a longer time.)

Multiple looping over metadata: you reiterate over result set metadata for each row in the result set. As you do not know the underlying implementation, this may be quite an expensive operation, there's no way to be sure. Thus, better do this once and cache the information you need.

Error handling: you catch everything, log it, and let the application continue running without any indication of an error, eventually based on incomplete data. Bad idea. Do not catch exceptions here, declare the method as "throws SQLException" (or maybe re-wrap it in MyOwnApplicationException or whatever) - just don't continue running in error conditions.

Multithreading: connections normally cannot be shared amongst multiple threads (eventually invalidating their underlying objects and doing all kinds of funny things.) I see no precautions to avoid multi-threaded access in the code.

General utility: ask yourself, what are you going to do with this map? Create objects from the contents? Create excel reports? Is your application code written to handle each and every funny datatype a driver implementor may be inclined to return via getObject()? I sincerely doubt this is really useful. For object mapping, have a look at various ORM frameworks / JPA, for reporting there are other frameworks. This looks like the re-invention of a wheel to me.