6

Yesterday multiple people on Stack recommended using try-with-resources. I am doing this for all my database operations now. Today I wanted to change Statement to PreparedStatement to make the queries more secure. But when I try to use a prepared statement in try-with-resources I keep getting errors like 'identifier expected' or ';' or ')'.

What am I doing wrong? Or isnt this possible? This is my code:

    try (Connection conn = DriverManager.getConnection(DBURL, DBUSER, DBPASS);
        PreparedStatement stmt = conn.prepareStatement("SELECT id FROM users WHERE id = ? LIMIT 1");
        stmt.setInt(1, user);
        ResultSet rs = stmt.executeQuery()) {

        // if no record found
        if(!rs.isBeforeFirst()) {
           return false;
        }
        // if record found
        else {
            return true;
        }

    } catch (SQLException e) {
        // log error but dont do anything, maybe later
        String error = "SQLException: " + e.getMessage() + "\nSQLState: " + e.getSQLState() + "\nVendorError: " + e.getErrorCode();
        return false;

    }
Dylan
  • 158
  • 1
  • 2
  • 10

3 Answers3

10

A try-with-resource statement is used to declare (Autoclosable) resources. Connection, PreparedStatement and ResultSet are Autoclosable, so that's fine.

But stmt.setInt(1, user) is NOT a resource, but a simple statement. You cannot have simple statements (that are no resource declarations) within a try-with-resource statement!

Solution: Create multiple try-with-resource statements!

try (Connection conn = DriverManager.getConnection(DBURL, DBUSER, DBPASS)) {
    executeStatement(conn);
} catch (SQLException e) {
    // log error but dont do anything, maybe later
    String error = "SQLException: " + e.getMessage() + "\nSQLState: " + e.getSQLState() + "\nVendorError: " + e.getErrorCode();
    return false;
}

private void executeStatement(Connection con) throws SQLException {
    try (PreparedStatement stmt = conn.prepareStatement("SELECT id FROM users WHERE id=? LIMIT 1")) {
        stmt.setInt(1, user);
        try (ResultSet rs = stmt.executeQuery()) {
            // process result
        }
    }
}

(Please note that technically it is not required to put the execution of the SQL statement into a separate method as I did. It also works if both, opening the connection and creating the PreparedStatement are within the same try-with-resource statement. I just consider it good practice to separate connection management stuff from the rest of the code).

isnot2bad
  • 24,105
  • 2
  • 29
  • 50
  • 1
    why? you can place the `Connection` and `PreparedStatement` inside the same resource creation block then uses `stmt` – Ross Drew Jul 24 '14 at 11:38
  • 2
    That's right, but usually in real world applications opening the JDBC connection and doing SQL work is/should be separated. Of course you could also have everything in one single method. – isnot2bad Jul 24 '14 at 11:39
  • 1
    But thanks for your remark. Added it to my answer to avoid confusion. – isnot2bad Jul 24 '14 at 11:43
1

try this code:

try (Connection conn = DriverManager.getConnection(DBURL, DBUSER, DBPASS)) {
     PreparedStatement stmt = conn.prepareStatement("SELECT id FROM users WHERE id = ? LIMIT 1");

        stmt.setInt(1, user);
        ResultSet rs = pstmt.executeQuery())

        // if no record found
        if(!rs.isBeforeFirst()) {
           return false;
        }
        // if record found
        else {
            return true;
        }

    } catch (SQLException e) {
        // log error but dont do anything, maybe later
        String error = "SQLException: " + e.getMessage() + "\nSQLState: " + e.getSQLState() + "\nVendorError: " + e.getErrorCode();
        return false;

    }

note that here, resource is your Connection and you have to use it in the try block ()

Ross Drew
  • 8,163
  • 2
  • 41
  • 53
Pat
  • 2,223
  • 4
  • 19
  • 25
  • 2
    really you should have the `PreparedStatement` inside the resource creation block as well so that it is closed properly. – Ross Drew Jul 24 '14 at 11:33
  • ok agreed. Also, found a post on the same topic at here: http://stackoverflow.com/questions/8066501/how-should-i-use-try-with-resources-with-jdbc May be someone can mark this one as duplicate. I don't have that privilege yet. – Pat Jul 24 '14 at 11:38
  • That post doesn't cover this specific problem however. – Ross Drew Jul 24 '14 at 11:39
  • @Ross I meant marking the question as duplicate. This question is asking about proper use of try-with-resources for JDBC, so as the one that I posted link to. – Pat Jul 24 '14 at 11:42
  • 1
    Also, the ResultSet is not closed when an exception is thrown. – Vlasec Jan 13 '15 at 14:05
1

Move

stmt.setInt(1, user);
ResultSet rs = stmt.executeQuery()

...within the try{ /*HERE*/ }

This is because stmt is the resource being created try (/*HERE*/) {} to be used try{ /*HERE*/ }

Try-with-resources

try (/*Create resources in here such as conn and stmt*/)
{
  //Use the resources created above such as stmt
}

The point being that everything created in the resource creation block implements AutoClosable and when the try block is exited, close() is called on them all. In your code stmt.setInt(1, user); is not an AutoCloseable resource, hence the problem.

Ross Drew
  • 8,163
  • 2
  • 41
  • 53