DD anomaly, and cleaning up database resources: is there a clean solution?

Question

Here's a piece of code we've all written:

    public CustomerTO getCustomerByCustDel(final String cust, final int del)
            throws SQLException {
        final PreparedStatement query = getFetchByCustDel();
        ResultSet records = null;
        try {
            query.setString(1, cust);
            query.setInt(2, del);
            records = query.executeQuery();

            return this.getCustomer(records);
        } finally {
            if (records != null) {
                records.close();
            }
            query.close();
        }
    }

If you omit the 'finally' block, then you leave database resources dangling, which obviously is a potential problem. However, if you do what I've done here - set the ResultSet to null outside the **try** block, and then set it to the desired value inside the block - PMD reports a 'DD anomaly'. In the documentation, a DD anomaly is described as follows:

DataflowAnomalyAnalysis: The dataflow analysis tracks local definitions, undefinitions and references to variables on different paths on the data flow.From those informations there can be found various problems. [...] DD - Anomaly: A recently defined variable is redefined. This is ominous but don't have to be a bug.

If you declare the ResultSet outside the block without setting a value, you rightly get a 'variable might not have been initialised' error when you do the if (records != null) test.

Now, in my opinion my use here isn't a bug. But is there a way of rewriting cleanly which would not trigger the PMD warning? I don't particularly want to disable PMD's DataFlowAnomalyAnalysis rule, as identifying UR and DU anomalies would be actually useful; but these DD anomalies make me suspect I could be doing something better - and, if there's no better way of doing this, they amount to clutter (and I should perhaps look at whether I can rewrite the PMD rule)

score 2 · Answer 1 · answered May 25 '12 at 23:26

I think this is clearer:

PreparedStatement query = getFetchByCustDel();
try {
    query.setString(1, cust);
    query.setInt(2, del);
    ResultSet records = query.executeQuery();
    try {
        return this.getCustomer(records);
    } finally {
        records.close();
    }
} finally {
    query.close();
}

Also, in your version the query doesn't get closed if records.close() throws an exception.

score 1 · Answer 2 · answered Sep 25 '13 at 06:56

I think that DD anomaly note is more bug, than a feature
Also, the way you free resources is a bit incomplete, for example

PreparedStatement pstmt = null;
Statement st = null; 

try {
    ...
} catch (final Exception e) {
    ...
} finally {
    try{
        if (pstmt != null) {
            pstmt.close();
        }
    } catch (final Exception e) {
        e.printStackTrace(System.err);
    } finally {
        try {
            if (st != null) {
                st.close();
            }
        } catch (final Exception e) {
            e.printStackTrace(System.err);
        }
    }
}

moreover this is not right again, cuz you should close resources like that

PreparedStatement pstmt = null;
Throwable th = null;

try {
    ...
} catch (final Throwable e) {
    <something here>
    th = e;
    throw e;
} finally {
    if (th == null) {
        pstmt.close();
    } else {
        try {
            if (pstmt != null) {
                pstmt.close();
            }
        } catch (Throwable u) {
        }
    }
}

DD anomaly, and cleaning up database resources: is there a clean solution?

2 Answers2