[FIXED] Is there a cleaner way to use try-with-resource and PreparedStatement?

Issue

Here is the Main.java:

package foo.sandbox.db;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;

public class Main {
    public static void main(String[] args) {
        final String SQL = "select * from NVPAIR where name=?";
        try (
                Connection connection = DatabaseManager.getConnection();
                PreparedStatement stmt = connection.prepareStatement(SQL);
                DatabaseManager.PreparedStatementSetter<PreparedStatement> ignored = new DatabaseManager.PreparedStatementSetter<PreparedStatement>(stmt) {
                    @Override
                    public void init(PreparedStatement ps) throws SQLException {
                        ps.setString(1, "foo");
                    }
                };
                ResultSet rs = stmt.executeQuery()
        ) {
            while (rs.next()) {
                System.out.println(rs.getString("name") + "=" + rs.getString("value"));
            }
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

And here is DatabaseManager.java

package foo.sandbox.db;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Statement;

/**
 * Initialize script
 * -----
 * CREATE TABLE NVPAIR;
 * ALTER TABLE PUBLIC.NVPAIR ADD value VARCHAR2 NULL;
 * ALTER TABLE PUBLIC.NVPAIR ADD id int NOT NULL AUTO_INCREMENT;
 * CREATE UNIQUE INDEX NVPAIR_id_uindex ON PUBLIC.NVPAIR (id);
 * ALTER TABLE PUBLIC.NVPAIR ADD name VARCHAR2 NOT NULL;
 * ALTER TABLE PUBLIC.NVPAIR ADD CONSTRAINT NVPAIR_name_pk PRIMARY KEY (name);
 *
 * INSERT INTO NVPAIR(name, value) VALUES('foo', 'foo-value');
 * INSERT INTO NVPAIR(name, value) VALUES('bar', 'bar-value');
 */
public class DatabaseManager {
    /**
     * Class to allow PreparedStatement to initialize parmaters inside try-with-resource
     * @param <T> extends Statement
     */
    public static abstract class PreparedStatementSetter<T extends Statement> implements AutoCloseable {
        public PreparedStatementSetter(PreparedStatement pstmt) throws SQLException {
            init(pstmt);
        }

        @Override
        public void close() throws Exception {
        }

        public abstract void init(PreparedStatement pstmt) throws SQLException;
    }

    /* Use local file for database */
    private static final String JDBC_CONNECTION = "jdbc:h2:file:./db/sandbox_h2.db;MODE=PostgreSQL";

    static {
        try {
            Class.forName("org.h2.Driver");  // Init H2 DB driver
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    /**
     * @return Database connection
     * @throws SQLException
     */
    public static Connection getConnection() throws SQLException {
        return DriverManager.getConnection(JDBC_CONNECTION, "su", "");
    }
}

I am using H2 database for simplicity since it's a file based one that is easy to create and test on.

So everything works and resources get cleaned up as expected, however I just feel there may be a cleaner way to set the PreparedStatement parameters from inside the try-with-resources block (and I don't want to use nested try/catch blocks as those look 'awkward'). Maybe there already exists a helper class in JDBC that does just this, but I have not been able to find one.

Preferably with a lambda function to initialize the PreparedStatement but it would still require allocating an AutoCloseable object so it can be inside the try-with-resources.

Solution

First off, your PreparedStatementSetter class is awkward:

• it is a typed class but the type is not used.
• the constructor is explicitly calling an overridable method, which is a bad practice.

Consider the following interface instead (inspired from the Spring interface of the same name).

public interface PreparedStatementSetter {
    void setValues(PreparedStatement ps) throws SQLException;
}

This interface defines a contract of what a PreparedStatementSetter is supposed to do: set values of a PreparedStatement, nothing more.

Then, it would be better to do the creation and initialization of the PreparedStatement inside a single method. Consider this addition inside your DatabaseManager class:

public static PreparedStatement prepareStatement(Connection connection, String sql, PreparedStatementSetter setter) throws SQLException {
    PreparedStatement ps = connection.prepareStatement(sql);
    setter.setValues(ps);
    return ps;
}

With this static method, you can then write:

try (
    Connection connection = DatabaseManager.getConnection();
    PreparedStatement stmt = DatabaseManager.prepareStatement(connection, SQL, ps -> ps.setString(1, "foo"));
    ResultSet rs = stmt.executeQuery()
) {
    // rest of code
}

Notice how the PreparedStatementSetter was written here with a lambda expression. That's one of the advantage of using an interface instead of an abstract class: it actually is a functional interface in this case (because there is a single abstract method) and so can be written as a lambda.

Answered By - Tunaki