JDBC Exception Handling

[keerthi matta]

Hi Friends,

I need help in resolving the best practice of handling exceptions in the given scenario,

Connection conn;
CallableStatement stmt;
ResultSet rs = null;
try {
// Code calling database to execute a StroedProc that results a ResultSet
//Exception while accessing ResultSet
} catch (SQLException) {
//handlingException
} finally {
try {
rs.close();
} catch(Exception) {
String msg = "Exception while closing ResultSet";
throw new UDException(msg);
} finally {
try {
stmt.close();
}catch(Exception) {
String msg = "Exception while closing Statement";
throw new UDException(msg);
} finally {
try {
conn.close();
} catch(Exception) {
String msg = "Exception while closing Connection";
throw new UDException(msg);
}
}
}

Is this right way of closing ResultSet, Statement and Connection. I need to throw user defined exception with specific message.

Thanks in advance,
Keerthi

[Layne Lund]

First of all, you should use the UBB CODE tags to preserve formatting in any code that you post here. It will make it much easier to read

Second, I don't think this code will compile. You have to give names to all the Exception references in the catch statment. For example, you need something like

try {
  // ...
} catch (Exception [B]ex[/B]) {
  // ...
}

Third, it's typically considered bad practice to catch Exception. One reason is that such a catch clause will also catch all RuntimeExceptions (such as NullPointerException, IllegalArgumentException, etc.) that should be allowed to propogate up the call stack so the user knows how to deal with them.

Fourth, throwing a new custom exception is usually a good idea. This avoids cluttering your own throws clauses (and the callers catch statements) with multiple exception types. However, your code hides the original cause of the exception. To include information about the original exception, you should use Exception Chaining. Java 1.4 and later implements this automatically with a constructor that takes a "cause" argument. So for example, you can do something like

try {
  // ...
} catch (Exception [B]ex[/B]) {
  String msg = "...";
  throw new UDException(msg, [B]ex[/B]);
}

Finally, it seems to me like you have a lot of try...catch statements in this code. In many places each try...catch statement only contains a single line of executable code. Personally, I would wrap the whole method, or at least groups of related statements, in a single try...catch statement. It looks like the main reason for the "extra" code is so you can give a descriptive error message about what the problem is. But if you use suggestion #4, it preserves the original cause so the descriptive error message isn't quite as important.

This is mostly my own opinion, especially, the last one so take it for the 2 cents that it's worth

Keep Coding!

Layne

[keerthi matta]

Hi Layne,

Thank you for the advice,
I have not posted my whole code previously. I am following all conventions u have listed out.
I am posting my code down here..
The requirement is
When ever we get an exception or not while closing a Statement, It should not stop us from closing a Connection.

I am following the below steps of closing my database resources. Please correct me if I am wrong in doing so..

/**
 * Clean up method to release all database resources used in respective
 * parent methods.
 * 
 * @param connConnection
 * @param stmtStatement
 * @param rsResultSet
 * @throws DaoException
 */
private void closeResources(Connection conn, Statement stmt, ResultSet rs)
throws DaoException {
final String PREFIX = new String("");
StringBuffer logMsg = new StringBuffer(PREFIX);
logMsg.append("Closing database resources..");
logger.debug(logMsg.toString());
try {
if (rs != null) {
rs.close();
}
} catch (SQLException sqlEx) {
logMsg.delete(0, logMsg.length());
logMsg.append(PREFIX);
logMsg.append("Error, encountered ");
logMsg.append(sqlEx);
logMsg.append(" while closing ResultSet.");
logger.error(logMsg.toString(), sqlEx);
throw new ARSDaoException(logMsg.toString(), sqlEx);
} finally {
try {
if (stmt != null) {
stmt.close();
}
} catch (SQLException sqlEx) {
logMsg.delete(0, logMsg.length());
logMsg.append(PREFIX);
logMsg.append("Error, encountered ");
logMsg.append(sqlEx);
logMsg.append(" while closing Statement.");
logger.error(logMsg.toString(), sqlEx);
throw new ARSDaoException(logMsg.toString(), sqlEx);
} finally {
try {
if (conn != null) {
conn.close();
}
} catch (SQLException sqlEx) {
logMsg.delete(0, logMsg.length());
logMsg.append(PREFIX);
logMsg.append("Error, encountered ");
logMsg.append(sqlEx);
logMsg.append(" while closing Connection.");
logger.error(logMsg.toString(), sqlEx);
throw new ARSDaoException(logMsg.toString(), sqlEx);
}
}
}
}

Thanks and Regards,
Keerthi

[Roger Chung-Wee]

Never throw an exception from your finally block, it will mask the exception thrown in the catch block.

I agree that you need to close all objects in order to be certain that you prevent problems such as memory leaks. What I do is to close the ResultSet, Statement and Connection objects in the finally block and write a message to the log if SQLException is thrown.