Use try-with-resources/close this "PreparedStatement" in a "finally" clause

Question

I'm running my JDBC code on sonarqube. There's issue with my code.

try (final Connection connection = DatabaseConnectionProvider.createConnection(configuration)) {
        PreparedStatement statement = connection.prepareStatement(
                "SELECT 1 FROM `todo_items` WHERE `id` = ? LIMIT 1;");
        statement.setLong(1, id);
        ResultSet resultSet = statement.executeQuery();
        if (!resultSet.next()) {
            return false;
        }
        statement = connection.prepareStatement("UPDATE `todo_items` SET `checked` = ? WHERE id = ?;");
        statement.setBoolean(1, checked);
        statement.setLong(2, id);
        statement.executeUpdate();
        return true;
    }

It says on line 3 and line 9 there is blocker issue.

"Use try-with-resources or close this "PreparedStatement" in a "finally" clause." I do not understand this.

You already have a `try-with-resource` for your connection. Put the prepared statement in there as well. — Federico klez Culloca, Mar 20 '21 at 12:34
You could do this problem much more simply, with less boilerplate, if you'd try JdbcTemplate. You're already using Spring. It's a mystery to me why you aren't using that class. You should be pooling your data sources and instantiating the pool in the Spring bean factory on startup. — duffymo, Mar 20 '21 at 13:57

rkosegi · Accepted Answer · 2021-03-20T12:45:04.523

You can go with inner try-with-resources, as you 2 distinct prepare statements inside:

try (final Connection connection = DatabaseConnectionProvider.createConnection(configuration)) {
    try(PreparedStatement statement = connection.prepareStatement("SELECT 1 FROM `todo_items` WHERE `id` = ? LIMIT 1;")) {
        statement.setLong(1, id);
        ResultSet resultSet = statement.executeQuery();
        if (!resultSet.next()) {
            return false;
        }
    }
    try(PreparedStatement statement = connection.prepareStatement("UPDATE `todo_items` SET `checked` = ? WHERE id = ?;")) {
        statement.setBoolean(1, checked);
        statement.setLong(2, id);
        statement.executeUpdate();
        return true;
    }
}

Small optimization is possible (note that 2 Autoclosables are present in try(.....):

try (final Connection connection = DatabaseConnectionProvider.createConnection(configuration);
        PreparedStatement statement = connection.prepareStatement("UPDATE `todo_items` SET `checked` = ? WHERE id = ?;")) {
    statement.setBoolean(1, checked);
    statement.setLong(2, id);
    return statement.executeUpdate() > 0;
}

You don't have to query for row existence in DB, as number of updated rows is usually returned by executeUpdate(). Actual return value might be JDBC driver implementation dependent (even though JDBC spec. is quite clear about it).

Use try-with-resources/close this "PreparedStatement" in a "finally" clause

1 Answers1