165

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;
}
dimwittedanimal
  • 656
  • 1
  • 13
  • 29
Jonas
  • 121,568
  • 97
  • 310
  • 388
  • 9
    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 it](https://docs.oracle.com/javase/7/docs/api/java/sql/ResultSet.html#close%28%29) – Alexander Farber Jul 12 '17 at 10:14
  • 6
    @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. – Basil Bourque Feb 14 '20 at 20:50

5 Answers5

204

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;
}
Jeanne Boyarsky
  • 12,156
  • 2
  • 49
  • 59
  • 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. – Basil Bourque Apr 15 '13 at 20:56
  • 29
    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.) – Hakanai May 08 '13 at 06:19
  • 1
    @Trejkaz good point on the possibility of not closing the PreparedStatement. I didn't think of that, but you are right! – Jeanne Boyarsky May 09 '13 at 23:51
  • I realize this is dependent on the instantiation order of Connection. Is it guaranteed the order in try-with-resources? – ArturoTena Sep 24 '13 at 21:33
  • 2
    @ArturoTena yes - the order is guaranteed – Jeanne Boyarsky Sep 25 '13 at 00:05
  • 2
    @JeanneBoyarsky is there another way to do this? If not I would need to create a specific createPreparedStatement method for each sql sentence – John Alexander Betts Oct 02 '13 at 16:30
  • @JohnB you could create a subclass to override the method. Or use a library like Spring JDBCTemplate that handles this for you. – Jeanne Boyarsky Oct 04 '13 at 02:03
  • @JeanneBoyarsky Could you elaborate on "you could create a subclass to override the method"? I don't quite get it. – Enrique Jan 02 '16 at 16:00
  • @JeanneBoyarsky does the current code still has this problem? – Jeremy S. Mar 08 '17 at 11:40
  • @JeanneBoyarsky the code above, and the problem mentioned by @Trejkaz `if an SQLException occurs while calling setInt` – Jeremy S. Mar 11 '17 at 09:40
  • @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. – Jeanne Boyarsky Mar 12 '17 at 00:41
  • 1
    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. – Florian F Aug 09 '17 at 20:26
  • 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`. – Shashank Garg Sep 12 '17 at 08:40
  • 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... – Welite Oct 17 '18 at 18:21
  • 1
    @Welite 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! – Jeanne Boyarsky Oct 28 '18 at 01:28
  • 1
    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? – Jeppz Jul 03 '19 at 09:24
104

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;
}
rogerdpack
  • 62,887
  • 36
  • 269
  • 388
bpgergo
  • 15,669
  • 5
  • 44
  • 68
  • 7
    How do you call [`Connection::setAutoCommit`](http://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#setAutoCommit-boolean-)? 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. – Basil Bourque Jul 31 '15 at 04:28
  • 1
    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 behavior – svarog Oct 14 '15 at 07:46
  • 1
    @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...) – rogerdpack Aug 17 '17 at 16:41
  • @rogerdpack Yes, that makes sense. Have your own implementation of [`DataSource`](https://docs.oracle.com/javase/8/docs/api/javax/sql/DataSource.html) where the `getConnection` method does as you say, get connection and configure it as needed, then passing on the connection. – Basil Bourque Aug 17 '17 at 18:19
  • 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. – adprocas Jan 09 '18 at 19:19
  • 1
    @rogerdpack thanks for the clarification in the answer. I have updated this to the selected answer. – Jonas Oct 24 '19 at 19:05
  • Should ps also be explicitly closed in the above code block, to avoid any memory leaks? – chaitra.kear Feb 08 '20 at 07:07
  • Such code triggers bug alert: Nullcheck of connection at line XXX of value previously dereferenced. For line with prepareStatement – Nikolay Klimchuk Dec 16 '22 at 23:48
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." );
    }
}
Andrei Volgin
  • 40,755
  • 6
  • 49
  • 58
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
  • 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? – Guilherme Taffarel Bergamin May 04 '22 at 11:59
  • 2
    @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. – Basil Bourque May 04 '22 at 16:15
  • 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 explanation – Guilherme Taffarel Bergamin May 04 '22 at 21:45
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();
}

VPK
  • 3,010
  • 1
  • 28
  • 35
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()) {
}
Miss Chanandler Bong
  • 4,081
  • 10
  • 26
  • 36
inder
  • 1,774
  • 1
  • 15
  • 15
  • 5
    This is more concise than the "classic approach" as described by @bpgergo ? I do not think so and the code is more difficult to understand. So please explain the advantage of this approach. – rmuller Oct 08 '16 at 19:04
  • 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. – djangofan Sep 10 '17 at 21:30
  • what if DriverManager.getConnection(JDBC_URL, prop); returns null? – Gaurav May 21 '18 at 12:53
  • 1
    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. – Sebsen36 Apr 12 '21 at 20:28