0

I have come across this pattern. Is it OK to reuse a Connection object within a single method when you need multiple SQL statements to execute?

My initial thought is to close all of the resources before moving on:

Connection conn = null;
PreparedStatement ps = null;
ResultSet rs = null;
try {
    conn = ConnectionFactory.getConnection();
    ps = conn.prepareStatement("SELECT * FROM MYTABLE WHERE COL=?");
    ps.setString(1, "val");
    rs = ps.executeQuery();
    // get values used to determine the next statement type
} catch (SQLException e) {
    LOG.error("SQL failed.", e);
} finally {
    if(rs != null){rs.close();}
    if(ps != null){ps.close();}
    if(conn != null){conn.close();}
}
// Then another SQL statement is needed (either an UPDATE or INSERT).
// Repeat the same pattern to open, use and close the connection

Is it just as safe to do the following? And if it is safe, is there a real benefit?

//... boilerplate
try {
    conn = ConnectionFactory.getConnection();
    ps = conn.prepareStatement("SELECT * FROM MYTABLE WHERE COL=?");
    ps.setString(1, "val");
    rs = ps.executeQuery();
    // ... more

    ps = conn.prepareStatement("UPDATE MYTABLE SET COL=?")
    // ... etc
} finally {
    if(rs != null){rs.close();}
    if(ps != null){ps.close();}
    if(conn != null){conn.close();}
}
Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
TCCV
  • 3,142
  • 4
  • 25
  • 30
  • 1
    If you're dealing with explicitly grabbing connections *at all*, then you're probably working at a lower level than makes sense. At a minimum, you should also be using try-with-resources. – chrylis -cautiouslyoptimistic- Sep 12 '18 at 19:26
  • Why would you *not* reuse a connection? – shmosel Sep 12 '18 at 19:28
  • I guess to make it clear, this is newly inherited code. In the code bases that I'm used to, I use hibernate when I need to interact with databases. Eventually this will be migrated - I'm just going through to identify areas to address in the meantime. – TCCV Sep 12 '18 at 20:19

2 Answers2

1

What you should do, is use try-with-resources:

//... boilerplate
try (Connection conn = ConnectionFactory.getConnection()) {
    try (PreparedStatement ps = conn.prepareStatement("SELECT * FROM MYTABLE WHERE COL=?")) {
        ps.setString(1, "val");
        try (ResultSet rs = ps.executeQuery()) {
            // ... more
        }
    }

    try (PreparedStatement ps = conn.prepareStatement("UPDATE MYTABLE SET COL=?")) {
        // ... etc
    }
}
Andreas
  • 154,647
  • 11
  • 152
  • 247
1

Reusing a connection is not an anti-pattern, it's entirely ok. Reusing the connection is the only way for both statements to get executed within the same local JDBC transaction. If you are writing applications that access relational databases you should learn about transactions.

The way you are implementing your exception handling is error-prone because if an exception is thrown when closing any resource the later resources don't get closed. If closing the preparedStatement throws an exception then the connection doesn't get closed. try-with-resources would be an improvement, but the way try-with-resources handles the edge cases makes me avoid it for JDBC in favor of nested try-finally blocks.

If there is an antipattern here it is using JDBC directly, it is very low-level, involves a lot of cut-and-paste, and doesn't make it easy to use transactions or connection pools. Using Spring would handle details like demarcating database transactions, using connection pools, and closing resources for you.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276