How should I use try-with-resources with JDBC?
Asked Answered
A

5

167

I have a method for getting users from a database with JDBC:

public List<User> getUser(int userId) {
    String sql = "SELECT id, name FROM users WHERE id = ?";
    List<User> users = new ArrayList<User>();
    try {
        Connection con = DriverManager.getConnection(myConnectionURL);
        PreparedStatement ps = con.prepareStatement(sql); 
        ps.setInt(1, userId);
        ResultSet rs = ps.executeQuery();
        while(rs.next()) {
            users.add(new User(rs.getInt("id"), rs.getString("name")));
        }
        rs.close();
        ps.close();
        con.close();
    } catch (SQLException e) {
        e.printStackTrace();
    }
    return users;
}

How should I use Java 7 try-with-resources to improve this code?

I have tried with the code below, but it uses many try blocks, and doesn't improve the readability much. Should I use try-with-resources in another way?

public List<User> getUser(int userId) {
    String sql = "SELECT id, name FROM users WHERE id = ?";
    List<User> users = new ArrayList<>();
    try {
        try (Connection con = DriverManager.getConnection(myConnectionURL);
             PreparedStatement ps = con.prepareStatement(sql);) {
            ps.setInt(1, userId);
            try (ResultSet rs = ps.executeQuery();) {
                while(rs.next()) {
                    users.add(new User(rs.getInt("id"), rs.getString("name")));
                }
            }
        }
    } catch (SQLException e) {
        e.printStackTrace();
    }
    return users;
}
Amorette answered 9/11, 2011 at 14:31 Comment(2)
In your second example, you don't need the inner try (ResultSet rs = ps.executeQuery()) { because A ResultSet object is automatically closed by the Statement object that generated itTautonym
@AlexanderFarber Unfortunately, there have been notorious problems with drivers that failed to close resources on their own. The School of Hard Knocks teaches us to always close all the JDBC resources explicitly, made easier using try-with-resources around Connection, PreparedStatement, and ResultSet too. No reason not to really, as the try-with-resources makes it so easy and makes our code more self-documenting as to our intentions.Vav
L
106

There's no need for the outer try in your example, so you can at least go down from 3 to 2, and also you don't need closing ; at the end of the resource list. The advantage of using two try blocks is that all of your code is present up front so you don't have to refer to a separate method:

public List<User> getUser(int userId) {
    String sql = "SELECT id, username FROM users WHERE id = ?";
    List<User> users = new ArrayList<>();
    try (Connection con = DriverManager.getConnection(myConnectionURL);
         PreparedStatement ps = con.prepareStatement(sql)) {
        ps.setInt(1, userId);
        try (ResultSet rs = ps.executeQuery()) {
            while(rs.next()) {
                users.add(new User(rs.getInt("id"), rs.getString("name")));
            }
        }
    } catch (SQLException e) {
        e.printStackTrace();
    }
    return users;
}
Lamprophyre answered 9/11, 2011 at 14:38 Comment(8)
How do you call Connection::setAutoCommit? Such a call is not allowed within the try between the con = and ps =. When getting a Connection from a DataSource that may be backed with a connection pool, we cannot assume how autoCommit is set.Vav
you would usually inject the connection into the method (unlike the ad-hoc approach shown in OP's question), you could use a connection managing class that will be called to provide or close a connection (be it pooled or not). in that manager you can specify your connection behaviorBoeotian
@BasilBourque you could move DriverManager.getConnection(myConnectionURL) into a method that also sets the autoCommit flag and returns the connection (or set it in the equivalent of the createPreparedStatement method in the preceding example...)Penetrating
@Penetrating Yes, that makes sense. Have your own implementation of DataSource where the getConnection method does as you say, get connection and configure it as needed, then passing on the connection.Vav
I like having that extra method to get the PreparedStatement. That way it can be reused if you're going to have the same, or a similar, PreparedStatement. In some cases (one that I'm working on) a generic method can be abstracted out and reused. Having a separate method means you can also name the method in a way that is descriptive, which increases readability. It also decreases the number of try blocks. With ctrl+click to go into the method in every IDE I use, the extra method really isn't an issue given the benefits. I also think the accepted answer looks cleaner. Just my two cents.Rummage
@Penetrating thanks for the clarification in the answer. I have updated this to the selected answer.Amorette
Should ps also be explicitly closed in the above code block, to avoid any memory leaks?Bini
Such code triggers bug alert: Nullcheck of connection at line XXX of value previously dereferenced. For line with prepareStatementTammy
P
207

I realize this was long ago answered but want to suggest an additional approach that avoids the nested try-with-resources double block.

public List<User> getUser(int userId) {
    try (Connection con = DriverManager.getConnection(myConnectionURL);
         PreparedStatement ps = createPreparedStatement(con, userId); 
         ResultSet rs = ps.executeQuery()) {

         // process the resultset here, all resources will be cleaned up

    } catch (SQLException e) {
        e.printStackTrace();
    }
}

private PreparedStatement createPreparedStatement(Connection con, int userId) throws SQLException {
    String sql = "SELECT id, username FROM users WHERE id = ?";
    PreparedStatement ps = con.prepareStatement(sql);
    ps.setInt(1, userId);
    return ps;
}
Pierrepierrepont answered 4/9, 2012 at 3:27 Comment(16)
When I tried this approach, Eclipse complained that the call to "con.prepareStatement" throws an unhandled exception (of type SQLException). Apparently a method called from the try-with-resources statement is not covered by the try block.Vav
No, it is covered, the problem is that the code above is calling prepareStatement from inside a method which doesn't declare to throw SQLException. Also, the code above has at least one path where it can fail without closing the prepared statement (if an SQLException occurs while calling setInt.)Mathur
@Trejkaz good point on the possibility of not closing the PreparedStatement. I didn't think of that, but you are right!Pierrepierrepont
I realize this is dependent on the instantiation order of Connection. Is it guaranteed the order in try-with-resources?Iotacism
@Iotacism yes - the order is guaranteedPierrepierrepont
@JeanneBoyarsky is there another way to do this? If not I would need to create a specific createPreparedStatement method for each sql sentenceArgil
@JohnB you could create a subclass to override the method. Or use a library like Spring JDBCTemplate that handles this for you.Pierrepierrepont
@JeanneBoyarsky Could you elaborate on "you could create a subclass to override the method"? I don't quite get it.Disenable
@JeanneBoyarsky does the current code still has this problem?Sikhism
@JeanneBoyarsky the code above, and the problem mentioned by @Trejkaz if an SQLException occurs while calling setIntSikhism
@JeremyS. Ah. No, it doesn't address the problem Trejkaz mentioned. In real life, I'd use a JDBC framework and never have this problem.Pierrepierrepont
Regarding Trejkaz's comment, createPreparedStatement is unsafe regardless how you use it. To fix it you would have to add a try-catch around the setInt(...), catch any SQLException, and when it happens call ps.close() and rethrow the exception. But that would result in a code almost as long and unelegant as the code the OP wanted to improve.Syringomyelia
Where would the class declaration Class.forName("com.mysql.jdbc.Driver"); go in try with resources approach? If I put it in try parentheses, it shows a syntax error. If I keep it above try block, It needs to be kept in another try/catch block to handle the classNotFoundException.Inceptive
This is absolutely the worst way how to list anything from database. Creating a new java.sql.Connection every time you call the getUser method is definitely not a good idea. Creating the new connection will not only take about 100ms (depending on db engine) on localhost but 500ms + at best on a remote db host...Jopa
@Jopa I agree that one should use a datasource. I was just trying to use try-with-resources in an example with databases. (because it is easy to create a connection leak). Pretend in my toy that it is only run once!Pierrepierrepont
What happens if ps.setInt throws an exception? the createPreparedStatement ps will not be assigned to ps in the try block so will it not remain open?Centipede
L
106

There's no need for the outer try in your example, so you can at least go down from 3 to 2, and also you don't need closing ; at the end of the resource list. The advantage of using two try blocks is that all of your code is present up front so you don't have to refer to a separate method:

public List<User> getUser(int userId) {
    String sql = "SELECT id, username FROM users WHERE id = ?";
    List<User> users = new ArrayList<>();
    try (Connection con = DriverManager.getConnection(myConnectionURL);
         PreparedStatement ps = con.prepareStatement(sql)) {
        ps.setInt(1, userId);
        try (ResultSet rs = ps.executeQuery()) {
            while(rs.next()) {
                users.add(new User(rs.getInt("id"), rs.getString("name")));
            }
        }
    } catch (SQLException e) {
        e.printStackTrace();
    }
    return users;
}
Lamprophyre answered 9/11, 2011 at 14:38 Comment(8)
How do you call Connection::setAutoCommit? Such a call is not allowed within the try between the con = and ps =. When getting a Connection from a DataSource that may be backed with a connection pool, we cannot assume how autoCommit is set.Vav
you would usually inject the connection into the method (unlike the ad-hoc approach shown in OP's question), you could use a connection managing class that will be called to provide or close a connection (be it pooled or not). in that manager you can specify your connection behaviorBoeotian
@BasilBourque you could move DriverManager.getConnection(myConnectionURL) into a method that also sets the autoCommit flag and returns the connection (or set it in the equivalent of the createPreparedStatement method in the preceding example...)Penetrating
@Penetrating Yes, that makes sense. Have your own implementation of DataSource where the getConnection method does as you say, get connection and configure it as needed, then passing on the connection.Vav
I like having that extra method to get the PreparedStatement. That way it can be reused if you're going to have the same, or a similar, PreparedStatement. In some cases (one that I'm working on) a generic method can be abstracted out and reused. Having a separate method means you can also name the method in a way that is descriptive, which increases readability. It also decreases the number of try blocks. With ctrl+click to go into the method in every IDE I use, the extra method really isn't an issue given the benefits. I also think the accepted answer looks cleaner. Just my two cents.Rummage
@Penetrating thanks for the clarification in the answer. I have updated this to the selected answer.Amorette
Should ps also be explicitly closed in the above code block, to avoid any memory leaks?Bini
Such code triggers bug alert: Nullcheck of connection at line XXX of value previously dereferenced. For line with prepareStatementTammy
V
10

As others have stated, your code is basically correct though the outer try is unneeded. Here are a few more thoughts.

DataSource

Other answers here are correct and good, such the accepted Answer by bpgergo. But none of them show the use of DataSource, commonly recommended over use of DriverManager in modern Java.

So for the sake of completeness, here is a complete example that fetches the current date from the database server. The database used here is Postgres. Any other database would work similarly. You would replace the use of org.postgresql.ds.PGSimpleDataSource with an implementation of DataSource appropriate to your database. An implementation is likely provided by your particular driver, or connection pool if you go that route.

A DataSource implementation need not be closed, because it is never “opened”. A DataSource is not a resource, is not connected to the database, so it is not holding networking connections nor resources on the database server. A DataSource is simply information needed when making a connection to the database, with the database server's network name or address, the user name, user password, and various options you want specified when a connection is eventually made. So your DataSource implementation object does not go inside your try-with-resources parentheses.

The purpose of DataSource is to externalize your database connection information. If you hard-code username, password, and such within your your source code, then a change to your database server configuration means having to recompile and redeploy your code — not fun. Instead, such database configuration details should be stored outside your source code, then retrieved at runtime. You can retrieve the configuration details via JNDI from a naming and directory server such as LDAP. Or you might retrieve from the Servlet container or Jakarta EE server running your app.

Nested try-with-resources

Your code makes proper use of nested try-with-resources statements.

Notice in the example code below that we also use the try-with-resources syntax twice, one nested inside the other. The outer try defines two resources: Connection and PreparedStatement. The inner try defines the ResultSet resource. This is a common code structure.

If an exception is thrown from the inner one, and not caught there, the ResultSet resource will automatically be closed (if it exists, is not null). Following that, the PreparedStatement will be closed, and lastly the Connection is closed. Resources are automatically closed in reverse order in which they were declared within the try-with-resource statements.

The example code here is overly simplistic. As written, it could be executed with a single try-with-resources statement. But in a real work you will likely be doing more work between the nested pair of try calls. For example, you may be extracting values from your user-interface or a POJO, and then passing those to fulfill ? placeholders within your SQL via calls to PreparedStatement::set… methods.

Syntax notes

Trailing semicolon

Notice that the semicolon trailing the last resource statement within the parentheses of the try-with-resources is optional. I include it in my own work for two reasons: Consistency and it looks complete, and it makes copy-pasting a mix of lines easier without having to worry about end-of-line semicolons. Your IDE may flag the last semicolon as superfluous, but there is no harm in leaving it.

Java 9 – Use existing vars in try-with-resources

New in Java 9 is an enhancement to try-with-resources syntax. We can now declare and populate the resources outside the parentheses of the try statement. I have not yet found this useful for JDBC resources, but keep it in mind in your own work.

ResultSet should close itself, but may not

In an ideal world the ResultSet would close itself as the documentation promises:

A ResultSet object is automatically closed when the Statement object that generated it is closed, re-executed, or used to retrieve the next result from a sequence of multiple results.

Unfortunately, in the past some JDBC drivers infamously failed to fulfill this promise. As a result, many JDBC programmers learned to explicitly close all their JDBC resources including Connection, PreparedStatement, and ResultSet too. The modern try-with-resources syntax has made doing so easier, and with more compact code. Notice that the Java team went to the bother of marking ResultSet as AutoCloseable, and I suggest we make use of that. Using a try-with-resources around all your JDBC resources makes your code more self-documenting as to your intentions.

Code example

package work.basil.example;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.time.LocalDate;
import java.util.Objects;

public class App
{
    public static void main ( String[] args )
    {
        App app = new App();
        app.doIt();
    }

    private void doIt ( )
    {
        System.out.println( "Hello World!" );

        org.postgresql.ds.PGSimpleDataSource dataSource = new org.postgresql.ds.PGSimpleDataSource();

        dataSource.setServerName( "1.2.3.4" );
        dataSource.setPortNumber( 5432 );

        dataSource.setDatabaseName( "example_db_" );
        dataSource.setUser( "scott" );
        dataSource.setPassword( "tiger" );

        dataSource.setApplicationName( "ExampleApp" );

        System.out.println( "INFO - Attempting to connect to database: " );
        if ( Objects.nonNull( dataSource ) )
        {
            String sql = "SELECT CURRENT_DATE ;";
            try (
                    Connection conn = dataSource.getConnection() ;
                    PreparedStatement ps = conn.prepareStatement( sql ) ;
            )
            {
                … make `PreparedStatement::set…` calls here.
                try (
                        ResultSet rs = ps.executeQuery() ;
                )
                {
                    if ( rs.next() )
                    {
                        LocalDate ld = rs.getObject( 1 , LocalDate.class );
                        System.out.println( "INFO - date is " + ld );
                    }
                }
            }
            catch ( SQLException e )
            {
                e.printStackTrace();
            }
        }

        System.out.println( "INFO - all done." );
    }
}
Vav answered 14/2, 2020 at 20:59 Comment(3)
In case the inner one throws an exception but there is no catch there, could it be captured by a catch on the outer one?Yaron
@GuilhermeTaffarelBergamin Yes, that’s how exceptions work in Java. If uncaught by the local code, they “bubble up” to the outer calling code. The bubbling up continues through all your called methods, until finally escaping your app and reaching the JVM for handling.Vav
Thank you. My question was because you don't need to declare a catch for a try with resources and it will execute an implicit finally, which means, this try has its own particularities. In my head, it could be ignoring the exception because there was no catch but there was a finally (which usually happens after catches). In any case, thanks for the explanationYaron
V
5

What about creating an additional wrapper class?

package com.naveen.research.sql;

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

public abstract class PreparedStatementWrapper implements AutoCloseable {

    protected PreparedStatement stat;

    public PreparedStatementWrapper(Connection con, String query, Object ... params) throws SQLException {
        this.stat = con.prepareStatement(query);
        this.prepareStatement(params);
    }

    protected abstract void prepareStatement(Object ... params) throws SQLException;

    public ResultSet executeQuery() throws SQLException {
        return this.stat.executeQuery();
    }

    public int executeUpdate() throws SQLException {
        return this.stat.executeUpdate();
    }

    @Override
    public void close() {
        try {
            this.stat.close();
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }
}


Then in the calling class you can implement prepareStatement method as:

try (Connection con = DriverManager.getConnection(JDBC_URL, prop);
    PreparedStatementWrapper stat = new PreparedStatementWrapper(con, query,
                new Object[] { 123L, "TEST" }) {
            @Override
            protected void prepareStatement(Object... params) throws SQLException {
                stat.setLong(1, Long.class.cast(params[0]));
                stat.setString(2, String.valueOf(params[1]));
            }
        };
        ResultSet rs = stat.executeQuery();) {
    while (rs.next())
        System.out.println(String.format("%s, %s", rs.getString(2), rs.getString(1)));
} catch (SQLException e) {
    e.printStackTrace();
}

Varipapa answered 5/12, 2012 at 16:1 Comment(1)
Nothing in the comment above ever says it doesn't.Mathur
B
3

Here is a concise way using lambdas and JDK 8 Supplier to fit everything in the outer try:

try (Connection con = DriverManager.getConnection(JDBC_URL, prop);
    PreparedStatement stmt = ((Supplier<PreparedStatement>)() -> {
    try {
        PreparedStatement s = con.prepareStatement("SELECT userid, name, features FROM users WHERE userid = ?");
        s.setInt(1, userid);
        return s;
    } catch (SQLException e) { throw new RuntimeException(e); }
    }).get();
    ResultSet resultSet = stmt.executeQuery()) {
}
Bedard answered 7/1, 2016 at 17:32 Comment(4)
This is more concise than the "classic approach" as described by @Lamprophyre ? I do not think so and the code is more difficult to understand. So please explain the advantage of this approach.Salmons
I don't think , in this case, that you are required to catch the SQLException explicitly. It is actually "optional" on a try-with-resources. No other answers mention this. So, you can probably simplify this further.Brodsky
what if DriverManager.getConnection(JDBC_URL, prop); returns null?Bourne
1. This is not more concise but a lot more confusing, 2. It also has the problem like the "extracted method answer" that the "PreparedStatement s" resource is still leaked and not closed, when there is an exception on the "s.setInt" call.Engobe

© 2022 - 2024 — McMap. All rights reserved.