10

Which is better for finally block:

finally {
        try {
            con.close();
            stat.close();
        } catch (SQLException sqlee) {
            sqlee.printStackTrace();
        }
    }

Or:

finally {
        try {
            if (con != null) {
                con.close();
            }
            if (stat != null) {
                stat.close();
            }
        } catch (SQLException sqlee) {
            sqlee.printStackTrace();
        }
    }
Rohit Jain
  • 209,639
  • 45
  • 409
  • 525
Sajad
  • 2,273
  • 11
  • 49
  • 92

5 Answers5

20

Better way to use is the 2nd one, because if an exception is thrown while initializing con or stat, they won't be initialized, and might be left initialized to null. In that case, using the 1st code will throw NullPointerException.

Also, if you are already on Java 7, you should consider using try-with-resources, which automatically closes the resources. From the linked tutorial:

The try-with-resources statement ensures that each resource is closed at the end of the statement. Any object that implements java.lang.AutoCloseable, which includes all objects which implement java.io.Closeable, can be used as a resource.

Rohit Jain
  • 209,639
  • 45
  • 409
  • 525
  • @Sajjad. Click on the link in the answer. That is oracle tutorial of try-with-resources. You can go through it. – Rohit Jain Aug 07 '13 at 22:40
  • Ok, Can we say that this `try-with-resources` is simpler than old way? Because that the specification of exception type is automatically and no need to catch block? – Sajad Aug 07 '13 at 22:42
  • @Sajjad. It is simpler in the sense, you don't have to worry about closing any resource you are using there. But you still need to provide a `catch` block. – Rohit Jain Aug 07 '13 at 22:43
  • @Sajjad. There is an example of `Connection` and `Statement`, in the tutorial, which you can use. – Rohit Jain Aug 07 '13 at 22:44
7

As of Java 7, you don't need any more use the finallyl block to close a Connection or Statement object. Instead you can make use of the new features called 'try-with-resources'.

First you declare a Connection and Statament objects by using the new syntax for a try-catch block as follows:

try(Connection con =  DriverManager.getConnection(database-url, user, password); Statement st = conn.createStatement()) {

 //your stuffs here
} catch (SQLException e) {
   e.printStackTrace();
}    

Doing so, you won't need to worry to close explicitly the linkage with the database in a finally block because the jvm will do it for you.

Have nice coding....

6

None of them are good enough. Use this:

public static void closeQuietly(AutoCloseable ... closeables) {
    for (AutoCloseable c : closeables) {
        if (c != null) {
            try {
                c.close();
            } catch (Exception e) {
                // log or ignore, we can't do anything about it really
            }
        }
    }
}

And call it like closeQuietly(stat, con);

Or use java 7's try-with-resource:

    List<String> results = new ArrayList<>();
    try (Statement statement = conn.createStatement();
         ResultSet rs = statement.executeQuery(query)) {

        int numberOfColumns = getColumnCount(rs);
        while (rs.next()) {
            int i = 1;
            while (i <= numberOfColumns) {
                results.add(rs.getString(i++));
            }
        }
    }
catch23
  • 17,519
  • 42
  • 144
  • 217
Xabster
  • 3,710
  • 15
  • 21
  • Quite a nice approach! But I still think that a `finally` block (that sets every `c = null;`) is desirable. – Barranka Aug 07 '13 at 22:41
  • You shouldn't manually set fields/variables to null to "notify" the GC unless the scope is long. In the above example we're talking a millisecond or less and the gain is low (if any). – Xabster Aug 07 '13 at 22:56
  • I answered this one too broad. In the above example, the original references to the Connection and Statement still exists outside this method (whoever called the method still has the original references) and we'd just be null'ing the copies of the references passed to our method. – Xabster Aug 07 '13 at 23:07
0

If there is a possibility either is null, you must check that. If the possibility does not exist, there is no valid reason to check for it.

Also, you can make your code slightly better readable by omitting some single-statement brackets:

finally {
    try {
        if (con != null)
            con.close();

        if (stat != null)
            stat.close();

    } catch (SQLException sqlee) {
        sqlee.printStackTrace();
    }
}
bas
  • 1,678
  • 12
  • 23
  • 2
    Although personal preference, I really dislike the omission of brackets for single statements. Especially, when they contain more single statements without brackets. I find it far harder to read code, and when indentation is inconsistent the readability is further reduced. – Muel Aug 07 '13 at 22:37
0

I would go with the second option, but adding a second nested finally block, just to make sure that both con and stat objects are marked for garbage collection:

finally {
    try {
        if(con != null)
            con.close();
        if(stat != null)
            stat.close();
    } catch(SQLException sqlee) {
        sqlee.printStackTrace();
    } finally {  // Just to make sure that both con and stat are "garbage collected"
        con = null;
        stat = null;
    }
}
Barranka
  • 20,547
  • 13
  • 65
  • 83