How can I handle this exception within this method?

Question

I have JDBC connection code similiar to the following from the Java JDBC tutorial:

public static void viewTable(Connection con) throws SQLException {
    Statement stmt = null;
    String query = "select COF_NAME, SUP_ID, PRICE, SALES, TOTAL from " + dbName + ".COFFEES";
    try {
      stmt = con.createStatement();
      ResultSet rs = stmt.executeQuery(query);
      while (rs.next()) {
        String coffeeName = rs.getString("COF_NAME");
        int supplierID = rs.getInt("SUP_ID");
        float price = rs.getFloat("PRICE");
        int sales = rs.getInt("SALES");
        int total = rs.getInt("TOTAL");
        System.out.println(coffeeName + "\t" + supplierID + "\t" + price + "\t" + sales + "\t" + total);
      }
    } catch (SQLException e ) {
      JDBCTutorialUtilities.printSQLException(e);
    } finally {
      stmt.close();
    }
  }

My problem with this way of handling to connection is that it closes the statement in the finally block and the method throws any SQLException that may occur. I don't want to do that, because I want any problems handled within this class. However, I do want that Statement#close() call in the finally block so that it is always closed.

Right now I'm placing this code in a separate method that returns a HashMap of the fields returned to that the exception is handled in-class. Is there another, possibly better way to handle this?

EDIT: The close() SQLException is the one I am concerned with. If possible I'd like to handle that within the method. I could write a try/catch in the finally, but that seems really awkward.

Answer 1

You have several problems:

• You need to close the ResultSet explicitly. Some drivers are less forgiving about forgetting to close the ResultSet than others, it doesn't hurt anything to be sure to close it.

• You ought to catch the SQLException thrown by the Statement.close, because it's not interesting and only serves to mask the interesting exception (if you have something in this method throw an exception, then the finally throws an exception on the way out, you get the exception from the finally block and lose the first exception). There really isn't anything that you can do if a close method call throws an exception, just log it and go on, it is not something to be concerned about.

• You should give up on the idea of handling all sqlexceptions in this method, the SQLException thrown by statement.executeQuery is the one that is worthwhile and it ought to be propagated if something goes wrong. It's likely other code in your application will want to know if your sql here succeeded or not and that's what throwing exceptions is for.

Personally I'd suggest using a library like Ibatis or spring-jdbc for this. JDBC is error-prone and tedious and it's better to take advantage of existing tools.

Answer 2

Here's how I usually handle this sort of resource stuff (note that whatever you do, you want a stmt != null check before you call stmt.close!):

SomeResource resource = null;
try {
    resource = /* ...get the resource... */;

    /* ...use the resource... */

    // Close it    
    resource.close();
    resource = null;

    // ...maybe do some post-processing... */

} catch (SomeException se) {
    // code to handle SomeException
} catch (SomeOtherException soe) {
    // code to handle SomeOtherException
} finally {
    if (resource != null) {
        try {
            resource.close();
        } catch (IOException e) {
        }
    }
}

...although my finally block is usually much simpler than that because I have utility methods to encapsulate it. (Specifically, it would probably look like this:

finally {
    resource = Utils.silentClose(resource);
}

...where silentClose does the !null check and close call masking any exception and always returns null.)

The key aspects of the above:

1. The resource is either open, or null, never !null but closed (other than briefly between the close call and the null; I'm assuming you won't be sharing this with other threads).
2. In the normal flow, I call close normally, without hiding any exception it may throw.
3. In the finally clause, if resource is !null, by definition some exception has occurred. Therefore, I should attempt to close resource, but prevent any exception from being thrown and masking the actual thing that went wrong.

In particular, I keep resource open only as long as I need it in the mainline code, for readability.

There are other idioms for this:

• The "rethrow" idiom: Always catch all exceptions, close your resources, and rethrow the exception. Leads to a lot of unnecessary code in my view.
• The "success flag" idiom: Set a flag — possibly your return value — that tells you whether things worked, and then always clean up in finally. The thing is, you get the same duplication there that you get with my code, unless you're always going to hide exceptions on close. Which brings us to:
• The "I don't care about exceptions on close" idiom: Always do the "silent" close. Gak. :-)

Applying the above to your code:

public static void viewTable(Connection con) throws SQLException {
    Statement stmt = null;
    ResultSet rs   = null; // <== You need this outside the try/catch block
    String query = "select COF_NAME, SUP_ID, PRICE, SALES, TOTAL from " + dbName + ".COFFEES";
    try {
        stmt = con.createStatement();
        rs = stmt.executeQuery(query);
        while (rs.next()) {
            String coffeeName = rs.getString("COF_NAME");
            int supplierID = rs.getInt("SUP_ID");
            float price = rs.getFloat("PRICE");
            int sales = rs.getInt("SALES");
            int total = rs.getInt("TOTAL");
            System.out.println(coffeeName + "\t" + supplierID + "\t" + price + "\t" + sales + "\t" + total);
        }

        // Explicit close, allows for exception since we won't be hiding anything
        rs.close();
        rs = null;
        stmt.close();
        stmt = null;

        // Possible further processing...

    } catch (SQLException e ) {
        JDBCTutorialUtilities.printSQLException(e);
    } finally {
        // Close the ResultSet first, then the Statement
        rs   = Utils.silentClose(rs);
        stmt = Utils.silentClose(stmt);
    }
}

Answer 3

There are many ways to write JDBC initialization and closing to avoid boiler plate. However to answer your question, you could wrap stmt.close() in a try-catch block as below. Also, you need to close your result set. (not written below) You may consider SpringDAO or Hibernate instead of JDBC to avoid checked exceptions.

public static void viewTable(Connection con) throws SQLException {
    Statement stmt = null;
    String query = "select COF_NAME, SUP_ID, PRICE, SALES, TOTAL from " + dbName + ".COFFEES";
    try {
      stmt = con.createStatement();
      ResultSet rs = stmt.executeQuery(query);
      while (rs.next()) {
        String coffeeName = rs.getString("COF_NAME");
        int supplierID = rs.getInt("SUP_ID");
        float price = rs.getFloat("PRICE");
        int sales = rs.getInt("SALES");
        int total = rs.getInt("TOTAL");
        System.out.println(coffeeName + "\t" + supplierID + "\t" + price + "\t" + sales + "\t" + total);
      }
    } catch (SQLException e ) {
      JDBCTutorialUtilities.printSQLException(e);
    } finally {
      try{
         stmt.close();
      }catch(Exception e) { /*LOG to indicate an issue */}
    }
  }

Answer 4

As the method stands it does whatever

 JDBCTutorialUtilities.printSQLException(e);

does when the excepotion occurs, unless that method rethrows the exception you will just return from the method with no lasting knowledge that the exception occurred.

You can put whatever code you like in the Exception block. The key question is what the caller of viewTable is supposed to do if an exception has happened.

You are presumably having code:

viewTable( /*etc*/);

doSomethingWith( price ); // for example

But that's no good if you've had an exception - price will not have been set. So either

a). in your exception block set a flag and then remember to check it

viewTable( /*etc*/);
if (itAllWorked)
     doSomethingWith( price ); // for example

which to me is error prone, and defeats the whole point of Exceptions. or

b). Don't catch the exception in viewTable (except maybe to log it, and rethrow, which I guess ma be what the utility methid is for).

try {

       viewTable()
       doSomethingWith(price):
       // all the normal flow
} catch (SqlException e) {
        //some reasnable action, which does not depend on things like proce
}