8
\$\begingroup\$

I have a function that works with a database. If something wrong happens I want to throw an exception, and I also want that all resources have to be closed.

Is this a good way to do it? There is a better way to do it?

public Strint getStuff(Integer id) throws Exception {

    String result = "";

    Connection connection = db.openConection();
    ResultSet rs = null;

    try {
       // DO STUFF WITH DB
    } catch (Exception e) {
        throw e;
    } finally {
        rs.close();
        db.closeConection(connection);
    }
    return result;
}
\$\endgroup\$
16
\$\begingroup\$
  1. You can skip catch part of try-catch-finally block
  2. You should check that rs is not null
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Also, since rs.close() and db.closeConnection(connection) can throw exceptions, each should be wrapped in a try/catch block. Otherwise, any exception they throw will suppress an exception from DO STUFF WITH DB (which is probably more important). (However, in Java 7, a suppressed exception can still be recovered, so this is less of an issue. You still don't want an exception from rs.close() to prevent closing the db connection.) \$\endgroup\$ – Ted Hopp Feb 29 '12 at 1:39
8
\$\begingroup\$

If you can, use Java 7's new try...catch.

try(NewResource res = new NewResource("Res1 closing")){
  res.doSomeWork("Listening to podcast");
} catch(Exception e){
  System.out.println("Exception: "+
  e.getMessage()+" Thrown by: "+e.getClass().getSimpleName());
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

You may verify the connection status in the finally or after that...

Something like this:

public Strint getStuff(Integer id) throws Exception {

    String result = "";

    Connection connection = db.openConection();
    ResultSet rs = null;

    try {
       // DO STUFF WITH DB
    } catch (Exception e) {
        throw e;
    }

    try{
        // Verify ResultSet status and close if open
        if(rs != null){
             rs.close();
        }
        // Verify the connection status and close if oppened
        if(((connection != null)) && (connection.isConnected())){
             connection.close();
        }
    }catch(Exception e){

    }

    return result;
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ The last try..catch should be inside of finally block of the first try..catch, or the connection and the ResultSet will remain open after the exception! \$\endgroup\$ – Manuel Romeiro Feb 17 at 9:51
1
\$\begingroup\$

Usually I end up writing methods like

public static void tryClose(ResultSet rs) {
    if (rs == null) { return; }
    try { rs.close(); } catch (SQLException e) { // log the exception just in case }
}

and put them in some utility or common base class, using them like this

Connection c = null;
ResultSet rs = null;
try {
    // your db code here
} finally {
    Base.tryClose(rs); // or possibly just tryClose(rs);
    Base.tryClose(c);  // or possibly just tryClose(c);
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Personally, I often use the following idiom:

public Strint getStuff(Integer id) throws Exception {

    Connection connection = db.openConection();
    try {
        ResultSet rs = connection.execute....();

        try {
           String result = "";
           // ... DO STUFF WITH DB ...
           return result;
        } finally {
           rs.close();
        }
    } finally {
        db.closeConection(connection);
    }
}

This works with all JDK's out there and reliably disposes of resources in event of an exception of any kind.

| improve this answer | |
\$\endgroup\$
-1
\$\begingroup\$

Instead of catching generic "Exception", its always better to catch specific exception and as @Andrew pointed its better to check for resultset not null.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ In this case, I think it doesn't make sense to catch specific exceptions. \$\endgroup\$ – svick Feb 27 '12 at 14:15
  • 1
    \$\begingroup\$ While generally true, this advice doesn't apply here. In this case he simply shouldn't be catching the exception. \$\endgroup\$ – Winston Ewert Feb 27 '12 at 17:31
  • \$\begingroup\$ why do you both think its not good to catch Specific exception ?Explain. \$\endgroup\$ – Sajan Chandran Feb 27 '12 at 20:54
  • \$\begingroup\$ @SajanChandran - There's no reason to catch a more specific exception because there's no reason to catch any exception at all. \$\endgroup\$ – Ted Hopp Feb 29 '12 at 1:41

Not the answer you're looking for? Browse other questions tagged or ask your own question.