HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

DAO Layer Code Review

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codedaolayerreview

Problem

Please review my following code for DAO layer.

```
public List getListOfChannels() throws DataAccessException {
// will have the channel in form of List
// to hold the channels list
List listChannels = null;
Connection conn = null;
Statement statement = null;
ResultSet resultSet = null;

try {
// get the db connection from pool
// this is DBCP lib on top doing this
conn = ManageConnections.getConnection();
statement = conn.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
ResultSet.CONCUR_READ_ONLY);

final String QUERY_STRING = "Select * from channel";
resultSet = statement.executeQuery(QUERY_STRING);

// is this good practice to put this ?
if (isResultSetEmptyOrNull(resultSet)) {
throw new DataAccessException(
"No more data of Channels found from db");
}

Channel channel = null;

listChannels = new ArrayList();

while (resultSet.next()) {
// get the object from the result set and

listChannels.add(channel);
}

log.debug("getListOfChannels Got the list of channels "
+ listChannels);

} catch (SQLException ex) {
throw new DataAccessException(SQL_EXCEPTION + ex, ex);
} catch (DBConnectionException ex) {
// re thrown, no logging
throw new DataAccessException(ex);
} catch (Exception ex) {
// Generic exception thrown , now throw Custom Exceptionn
throw new DataAccessException(GENERIC_EXCEPTION + ex, ex);
} finally {
try {
if (conn != null) {
// this is DBCP lib on top doing this
ManageConnections.close(conn);
}
if (statement != null) {

Solution

There are a couple of improvements that can be made here:

  • a) your QUERY_STRING is magic string. Move it to a different place,


because it make your code difficult to maintain

  • user linked list


instead of array list. It has O(1) on insertion in worst case,
whereas array list has O(n)

  • do not rethrow exceptions in finally


block. You will lose all details of your original exception

  • every time when you concatenate strings with "+" you make too many


operations with memory, because strings are immutable. use *.format
instead

Context

StackExchange Code Review Q#23609, answer score: 3

Revisions (0)

No revisions yet.