虽然接受的答案解决了问题的根本原因，但实际实施存在一些问题。
-
Exceptions被吞了
-
PreparedStatement 应该独立于 Connection 关闭
-
ResultSet 应该独立于 PreparedStatement 或 Connection 关闭
- 它不处理
TYPE_FORWARD_ONLY ResultSets 在 ResultSet.next() 为空时抛出的问题
- 它不检查结果是否唯一
- 它没有正确的日志记录
一个更完整的例子如下：
package com.stackoverflow.questions.Q42327984;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Optional;
import javax.sql.DataSource;
import lombok.Value;
import lombok.extern.log4j.Log4j2;
@SuppressWarnings("javadoc")
public class Answer {
/**
* <p>
* We use lombok's {@link Log4j2} annotation to create a Log4j2 logger, avoid using {@link System#out},
* {@link System#err} or {@link Throwable#printStackTrace()}
* </p>
*
* @See Lombok's <a href="https://www.projectlombok.org/features/log">log</a> api.
* @See Log4j2's install <a href="https://logging.apache.org/log4j/2.x/maven-artifacts.html">guide</a>.
*/
@Log4j2
static class UserDao {
final DataSource dataSource;
final UserQueries queries;
public UserDao(final DataSource dataSource, final UserQueries queries) {
this.dataSource = dataSource;
this.queries = queries;
}
/**
* Find {@link User} by beam id.
* <p>
* In most systems not finding a User would not be an "exceptional" condition, we should return {@link Optional}
* instead of throwing an {@link Exception}
* </p>
* <p>
* 'get' is fine for bean like things, but 'find' is for queries
* </p>
* <p>
* We add "ByBeamId" so we can disambiguate between this method and other queries that might also find
* {@link User} by a int value.
* </p>
*
* @param beamID the beam ID
* @return the {@link Optional} {@link User}, null's are the "billion dollar mistake"
* @throws {@link NoSuchUserException} the no such user exception
*/
public Optional<User> findUserByBeamId(int beamID) throws NoSuchUserException {
log.debug("findUserByBeamId({})", beamID);
final String userByBeamID = this.queries.getUserByBeamID();
log.debug("findUserByBeamId({}) userByBeamID query: {}", beamID, userByBeamID);
try (final Connection conn = this.dataSource.getConnection();
final PreparedStatement stmt = conn.prepareStatement(userByBeamID)) {
stmt.setInt(1, beamID);
/*
* ResultSets should be closed properly also
*/
try (final ResultSet rs = stmt.executeQuery()) {
/*
* There are other ways to find if the ResultSet is empty but the method below is safe and
* can handle most implementations
*/
int resultSetType = rs.getType();
try {
boolean scrolledForwardToRow = rs.next();
if (!scrolledForwardToRow) {
return Optional.empty();
}
} catch (SQLException e) {
/*
* ResultSets of TYPE_FORWARD_ONLY can throw a SQLException on ResultSet.next() if the ResultSet
* is
* empty.
*/
if (ResultSet.TYPE_FORWARD_ONLY == resultSetType) {
log.debug(
"findUserByBeamId({}): initial rs.next() call failed but ResultSet is TYPE_FORWARD_ONLY so returning empty.",
userByBeamID, e);
return Optional.empty();
}
log.error("findUserByBeamId({}): initial rs.next() call failed, return empty.", userByBeamID,
e);
throw e;
}
/*
* User should not have a reference to its DAO, this is a recipe for trouble.
*
* In general User should be an interface and a static factory method or a builder should be used
* for construction.
* 'new' should be avoided as it tightly couples a specific implementation of User.
*
* User user = new User(this,
* rs.getInt("beamID"),
* rs.getString("name"),
* rs.getInt("points"),
* rs.getInt("time"));
*/
/*
* Assigning to local variables has a couple advantages:
* - It makes stepping through the debugger much easier.
* - When an exception occurs the line number in the stack trace will only contain one method.
*
* Note, that the performance impact of local variables is not significant in a method of this
* complexity with so many external calls.
*/
final int beamIDFromRs = rs.getInt("beamID");
final String name = rs.getString("name");
final int points = rs.getInt("points");
final int time = rs.getInt("time");
final User user = User.of(beamIDFromRs, name, points, time);
/*
* Before we return our result we need to make sure it was unique.
*
* There are other ways to find if the ResultSet has no more row but the method below is safe
* can handle most implementations.
*/
try {
boolean scrolledForward = rs.next();
if (!scrolledForward) {
return Optional.of(user);
}
} catch (SQLException se) {
/*
* ResultSets of TYPE_FORWARD_ONLY can throw a SQLException on ResultSet.next() if the ResultSet
* is
* empty.
*/
if (ResultSet.TYPE_FORWARD_ONLY == resultSetType) {
log.debug(
"findUserByBeamId({}): rs.next() call failed but ResultSet is TYPE_FORWARD_ONLY so returning user: {}.",
userByBeamID, user, se);
return Optional.of(user);
}
log.error("findUserByBeamId({}): rs.next() call failed.", userByBeamID, se);
throw se;
}
log.error("findUserByBeamId({}): results not unique.", userByBeamID);
throw new IllegalStateException("findUserByBeamId(" + beamID + ") results were not unique.");
}
} catch (SQLException se) {
log.error("findUserByBeamId({}): failed", userByBeamID, se);
throw new IllegalStateException("findUserByBeamId(" + beamID + ") failed", se);
}
}
/**
* Gets the user.
*
* @param beamID the beam ID
* @return the {@link User}
* @throws NoSuchUserException if the user does not exist.
* @deprecated use {@link #findUserByBeamId(int)}
*/
@Deprecated
public User getUser(final int beamID) throws NoSuchUserException {
return findUserByBeamId(beamID).orElseThrow(() -> new NoSuchUserException(beamID));
}
}
interface UserQueries {
String getUserByBeamID();
}
/**
* Use interfaces to avoid tight coupling
*/
interface User {
/**
* static factory method for creating User instances
*
* @param beamID the beam ID
* @param name the name
* @param points the points
* @param time the time
* @return the user
*/
static User of(final int beamID, final String name, final int points, final int time) {
return new DefaultUser(beamID, name, points, time);
}
/**
* Gets the beam ID.
*
* @return the beam ID
*/
int getBeamID();
/**
* Gets the name.
*
* @return the name
*/
String getName();
/**
* Gets the points.
*
* @return the points
*/
int getPoints();
/**
* Gets the time.
*
* @return the time
*/
int getTime();
}
/**
* <p>
* We use lombok's {@link Value} to create a final immutable object with an all args constuctor
* ({@link #DefaultUser(int, String, int, int)} and proper
* {@link #toString()}, {@link #equals(Object)} and {@link #hashCode()} methods
* </p>
*
* @See Lombok's <a href="https://www.projectlombok.org/features/Value">value</a> api.
*/
@Value
static class DefaultUser implements User {
final int beamID;
final String name;
final int points;
final int time;
}
/**
* The Class NoSuchUserException.
*/
@SuppressWarnings("serial")
public static class NoSuchUserException extends RuntimeException {
final int beamID;
public NoSuchUserException(int beamID) {
this.beamID = beamID;
}
/**
* Gets the beam ID.
*
* @return the beam ID
*/
public int getBeamID() {
return this.beamID;
}
}
}