CODEWITHSUNDEEP
javadatabasejdbcexception
RockmanJoe
RockmanJoe

Reputation: 1658

Is there a better way to handle an Exception thrown from some DataSource?

I currently am working on a project that does a lot of work with Database.

One core idiom that I have reused many, many times in my code is the following.

My question is, is there a better way to handle the exceptions at each step of the getTransformedResults method? Is this a proper way of handling the SQLExceptions, or is there a better, more concise way of doing this?

Thanks for your input!

public ResultType handleResultSet(ResultSet rs);

public ResultType getTransformedResults(String query) throws SQLException {
  ResultType resultObj = new ResultType();

  Connection connection = null;
  try { 
    connection = dataSource.getConnection();
  } catch (SQLException sqle) {
    // cleanup 
    throw sqle;
  }

  Statement stmt = null;
  try { 
    stmt = connection.createStatement();
  } catch (SQLException sqle) {
    try { connection.close() } catch (SQLException dontCare) {}
    // cleanup
    throw sqle;
  }

  ResultSet rs = null;
  try { 
    ResultSet rs = stmtm.executeQuery(query);
    resultObj = handleResultSet(rs);
  } catch (SQLException sqle) {
    // cleanup
    throw sqle;
  } finally {
    if (rs != null) try { rs.close() } catch (SQLException dontCare) {}
    try { stmt.close() } catch (SQLException dontCare) {}
    try { connection.close() } catch (SQLException dontCare) {}
  }

  return resultObj;
}

Upvotes: 0

Views: 916

Answers (5)

Yuriy Nakonechnyy
Yuriy Nakonechnyy

Reputation: 3842

You may be interested in using Apache Commons DbUtils which is aimed exactly at such purposes.

It has some drawbacks when trying to use more sophisticated JDBC but for regular usage it should be more than enough.

Besides that, your code contains too much try/catch blocks and can be simplified to something like the following: 

public interface ResultSetHandler<ResultType> {
     ResultType handleResultSet(ResultSet rs);
}

public <ResultType> ResultType getTransformedResults(String query, ResultSetHandler<ResultType> rsh) throws SQLException {      
  Connection connection = null;
  Statement stmt = null;

  try { 
    connection = dataSource.getConnection();
    stmt = connection.createStatement();
    ResultSet rs = stmtm.executeQuery(query);
    return rsh.handleResultSet(rs);
  } catch (SQLException sqle) {
    // cleanup 
    throw sqle;
  } finally {
    if(stmt != null) {
      statement.close(); // closes also resultSet
      connection.close();
    }
  }
}

Though Apache Commons DbUtils library does exactly the same under the hood.

Upvotes: 0

Matt
Matt

Reputation: 11805

Connection c = null;
Statement s = null;
ResultSet r = null;

try {
  c = datasource.getConnection();
  s = c.createStatement();
  r = s.executeQuery(sql);
  rsh.handleResultSet(r);
}
finally {
  DbUtils.closeQuietly(r);
  DbUtils.closeQuietly(s);
  DbUtils.closeQuietly(c);
}

Note that DbUtils is apaache commons-dbutils, and the closeQuietly is equivalent to:

try {
  c.close();
}
catch (SQLException e) {
}

This all being said, i'd recommend using spring's jdbc features:

JdbcTemplate template = new JdbcTemplate(dataSource);
List data = template.query(sql, new RowMapper() { ... });

The RowMapper is an interface whose implementation has the job of converting the current position in the resultset to an object. So by simply giving it the logic of what to do with one row, you automatically collect the list of the objects for all rows in these two lines of code plus whatever it takes to map the row. There's other methods which let you work with the ResultSet in different ways, but this is a pretty standard way in which people use it.

All the connection and statement management is done for you, and you don't have to worry about resource management at all.

Upvotes: 0

Dave Newton
Dave Newton

Reputation: 160181

Personally I might handle this by passing in an interface implementation rather than subclassing.

Ultimately, if you're only handling the exceptions in that method, and not polluting the mainline code, what else can you really do, and what would be the point of doing it? You might make each step a bit more granular so it's not all in one method, but other than that...

You might consider an application-specific exception, which may make testing and configuration cleaner, but that depends on context.

Clarification of interface idea

Instead of subclassing you'd have an interface that implemented the handling of result sets and query string retrieval, so two methods--one for the query, one for the results.

You'd pass an implementation to an instance of mostly what you have now, but it takes the interface instead of a query string. The rest of the code is essentially identical, but it gets the query string from the interface impl, and calls the interface impl's result handling method, saving the result until the cleanup.

It's essentially the same as you have now, but IMO cleaner since any class could implement the interface, including anonymous classes, or other classes in your domain.

Upvotes: 1

Arvik
Arvik

Reputation: 2614

org.springframework.jdbc.core.JdbcTemplate - "...simplifies the use of JDBC and helps to avoid common errors."

Upvotes: 0

Bill K
Bill K

Reputation: 62759

Java 7 has some constructs you might appreciate, I think you can use try/finally without catch (which mimics your catch and rethrow).

Also, since you've caught and handled the SQL exception, perhaps you should re-throw it as something else--perhaps as a runtime exception--this makes it easier to catch all runtime exceptions at a primary entry point rather than having to deal with exceptions every single time you access the DB.

Upvotes: 1

Related Questions