What's the best practice for JDBC connection/resultset/statement

I am wondering what is the best practice for the following code snippet, do I have to close all stmt and rs everytime the executeQuery is done inside each 'if' or can I do just like now, close rs a开发者_高级运维nd stmt at the end of runOnSqlServer, and close connection at the end fo run method? Thanks for any pointers!

public void runOnSqlServer(Connection con, String[] params, String db){
    try{
    Statement stmt = con.createStatement();
    ResultSet rs = null;
    if(isVer){
        rs = stmt.executeQuery(micro_verSql);
        commonAct(rs, getParameter("isVer"), 1);
    } 
    if(isInfo){
        rs = stmt.executeQuery("SELECT DATABASEPROPERTYEX('"+db+"', 'COLLATION')");
        commonAct(rs, getParameter("isInfo"), 1);
    }
    }catch(SQLException){
    .....
    }finally{
    stmt.close();
    rs.close();
    }
}

public void run(CommandContext ctx) {
    try{
        ...
        runOnSqlServer(con, params, sqldb);
        ...
    }catch(Exception ex){
    }finally{
         if (con != null) con.close();
    }
}

IMHO, you should close the statement and result set after the method has completed (in the finally) but close the ResultSet everytime you re-user it also you should ALWAYS close the connect when you are finished with it.

edit: re-initialise

ResultSet set = statement.executeQuery();
set = statement2.executeQuery();

you are setting the ResultSet "set" to a new result set. The original set is now not pointing to anything but still open waiting to be collected.

You should close all database resources:

1. in the method scope in which they were created.
2. in reverse order of creation
3. in the finally block
4. individually wrapped in try/catch blocks.

I think you should consider reading this articles under "Best practices to improve performance in JDBC".

• Optimization with Connection
• Optimization with Statement
• Optimization with ResultSet

As of Java 7 you can use try-with-resources, which clearly marks the scope of your Connection, Statement and ResultSet and automatically closes them. Your method runOnSqlServer(…) could look like this:

public void runOnSqlServer(Connection con, String[] params, String db){
    try (Statement stmt = con.createStatement()) {
        if(isVer){
            try (ResultSet rs = stmt.executeQuery(micro_verSql)) {
                commonAct(rs, getParameter("isVer"), 1);
            } // rs is closed here
        } 
        if(isInfo){
            try (ResultSet rs = stmt.executeQuery("SELECT DATABASEPROPERTYEX('"+db+"', 'COLLATION')")) {
                commonAct(rs, getParameter("isInfo"), 1);
            } // rs is closed here
        }
    } // stmt is closed here
    catch(SQLException){
        .....
    }
    // finally not necessary, as rs and stmt are closed automatically
}

As a side note: Don't use constructs like "SELECT DATABASEPROPERTYEX('"+db+"', 'COLLATION')" unless you are sure db is not derived from user input. Concatenating strings to build a SQL statement is vulnerable to SQL injection. Use PreparedStatement instead.

As others have mentioned, using finally to ensure items are closed is very important.

Also, consider using a connection pool to mitigate the cost of frequently opening and closing connections. c3p0 is one such package.