Search code examples
javadata-access-layer

java data access: is this good style of java data access code, or is it too much try finally?


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;
}

Solution

  • 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.