dreadedmonkeygod . net

Q: What's wrong with this code?

public String[] getCustInfo(String custId) {
    try {
        Connection c = getConnection();
        CallableStatement cs =
            c.prepareCall("CALL GETCUSTINFO(?)");
        cs.setInt(1, Integer.parseInt(custId));
        ResultSet rs = cs.executeQuery();

        String[] data = new String[9];
        for (int i = 0; i < data.length; i++) {
            data[i] = rs.getString(i);
        }

        return data;
    } catch (Exception e) {
        return null;
    }
}

A: It looks like it was written by a freshman business major who took a Java class to fulfill his math requirement, and got in over his head.

If your answer matched the above, please continue coding. If, on the other hand, you said, "Hey! That looks like my code," kindly die.

In the interest of full disclosure, a sample of how I usually write this kind of code is included after the jump.

My revision (leaving out the JavaDocs, which you should most certainly put in):

public Customer getCustomer(int custId)
    throws DataAccessException
{
    Connection conn = null;
    CallableStatement call = null;
    ResultSet rs = null;
    
    try {
        conn = getConnection();
        call = conn.prepareCall("CALL GETCUSTINFO(?)");
        call.setInt(1, custId);
        rs = call.executeQuery();
        if (!rs.next()) throw new DataAccessException("No customer found with id \"" + custId + "\"");
        return populateCustomer(rs);
    }
    catch (Exception e) {
        throw new DataAccessException("Error finding customer with id \"" + custId + "\": " + e, e);
    }
    finally {
        try { rs.close() }
        catch (Exception e) { log(e); }
        
        try { call.close(); }
        catch (Exception e) { log(e); }

        try { conn.close(); }
        catch (Exception e) { log(e); }
    }
}

private Customer populateCustomer(ResultSet rs)
    throws SQLException
{
    Customer cust = new Customer();
    cust.setCustId(rs.getInt("CUST_ID"));
    cust.setName(rs.getString("NAME"));
    // ...etc.

    return cust;
}