Is this good style of java data access code, or is it too much try finally ?
public List<Item> getItems() throws ItemException {
ArrayList<Item> items = new ArrayList<Item>();
try {
Connection con = ds.getConnection();
try {
PreparedStatement pStmt = con.prepareStatement("SELECT ....");
try {
ResultSet rs = pStmt.executeQuery();
try {
while (rs.next()) {
Item item = new Item();
item.setItemNo(rs.getString("item_id"));
// ...
items.add(item);
}
} finally {
rs.close();
}
} finally {
pStmt.close();
}
} finally {
con.close();
}
} catch (SQLException e) {
throw new ItemException(e);
}
return items;
}
Compare it to my code:
Connection con = null;
PreparedStatement pStmt = null;
ResultSet rs = null;
try
{
con = ds.getConnection();
pStmt = con.prepareStatement("SELECT ....");
rs = pStmt.executeQuery();
while (rs.next()) {
Item item = new Item();
item.setItemNo(rs.getString("item_id"));
...
items.add(item);
}
}
finally {
rs = DBUtil.close (rs);
pStmt = DBUtil.close (rs);
con = DBUtil.close (rs);
}
Here is what close()
looks like:
public static ResultSet close (ResultSet rs) {
try {
if (rs != null)
rs.close ();
} catch (SQLException e) {
e.printStackTrace ();
// Or use your favorite logging framework.
// DO NOT THROW THIS EXCEPTION OR IT WILL
// HIDE EXCEPTIONS IN THE CALLING METHOD!!
}
return null; // Make sure no one uses this anymore
}
[EDIT] You'll need to copy this code for the other types.
I also moved all this into a helper class called DBOp
so I just have to override processRow(ResultSet row)
to do the actual processing and I can omit all that boilerplate code. In Java 5, the constructor of DBOp
reads:
public DBOp (Logger log, DataSource ds, String sql, Object... param)
I'm passing in the logger so I can show which instance is actually polling data.