[JDBC 라이브러리 구현하기 - 1단계] 아벨(신준혁) 미션 제출합니다. #329
Conversation
Signed-off-by: JunH <[email protected]>
|
|
||
| private final DataSource dataSource; | ||
| private final RowMapper<User> userRowMapper = resultSet -> new User( |
There was a problem hiding this comment.
row mapper를 static으로 사용하는 것은 어떻게 생각하시는지 궁금합니다!
There was a problem hiding this comment.
static을 사용해도 무방하다는 생각은 하는데 한편으론 굳이라는 생각도 듭니다. 나중에 미션의 단계가 올라가면서 어떻게 리팩토링이 진행될 지 모르겠지만, 싱글톤으로 계층 관련된 클래스들이 관리가 된다면 static은 안붙여도 된다고 생각합니다.
|
|
||
| public class UserHistoryDao { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(UserHistoryDao.class); | ||
| private static final RowMapper<UserHistory> userHistoryRowMapper = resultSet -> new UserHistory( |
There was a problem hiding this comment.
여기서는 static을 사용해 주셨네요 혹시 UserDao와 다르게 한 이유가 있을까요?
There was a problem hiding this comment.
그러게요. 위에 static이 붙은 log와 같이 적으면서 좀 헷갈렸던 것 같습니다. static은 일단 삭제하도록 하겠습니다 :)
| public UserHistory findLogByUser(final User user) { | ||
| log.debug("User history id : {}", user.getId()); | ||
| final var sql = "select * from user_history where user_id = ?"; | ||
| return jdbcTemplate.queryForObject(sql, userHistoryRowMapper, user.getId()); |
| @@ -14,4 +20,64 @@ public class JdbcTemplate { | |||
| public JdbcTemplate(final DataSource dataSource) { | |||
| this.dataSource = dataSource; | |||
| } | |||
|
|
|||
| public <T> List<T> query(final String sql, final RowMapper<T> rowMapper, final Object... params) { | |||
| try (final var connection = requireNonNull(dataSource, "dataSource가 null입니다.").getConnection(); | |||
There was a problem hiding this comment.
Connection의 null 처리를 이렇게 하니 깔끔해 보여서 좋습니다!
| try (final var resultSet = preparedStatement.executeQuery()) { | ||
| log.info("JDBC QUERY SQL = {}", sql); | ||
| return getResults(rowMapper, resultSet); | ||
| } |
There was a problem hiding this comment.
현재 try 안에 다시 try 블록이 보이는데요, 어차피 발생한 예외라면 33번째 줄의 catch 블록에서 잡아주지 않을까 생각되는데 아벨의 생각은 어떠신가요?
There was a problem hiding this comment.
아 이 부분은 final 블록을 쓰지 않고 resultSet을 자동으로 close해보려고 한건데, 엔델이 보시기엔 이게 가독성이 어떻게 보일지 궁금합니다.
There was a problem hiding this comment.
아하 result Set을 자동으로 닫기 위해 try를 사용하신거군요.
중첩 단계가 높아질수록 가독성이 개인적으로 안좋아진다고 생각하긴 하는데 자동으로 자원을 close 하는 장점이 더 큰 것 같네요.
| try (final var resultSet = preparedStatement.executeQuery()) { | ||
| if (resultSet.next()) { | ||
| log.info("JDBC QUERY_FOR_OBJECT SQL = {}", sql); | ||
| return rowMapper.mapRow(resultSet); | ||
| } | ||
| } |
| } | ||
|
|
||
| public <T> T queryForObject(final String sql, final RowMapper<T> rowMapper, final Object... params) { | ||
| try (final var connection = requireNonNull(dataSource).getConnection(); |
There was a problem hiding this comment.
이부분은 다른 부분처럼 "dataSource가 null입니다."라는 메시지가 없는데 의도된 것일까요?
There was a problem hiding this comment.
이 부분은 제가 빼먹은 것 같네요. 추가하도록 하겠습니다! :)
| } | ||
|
|
||
| public void update(final String sql, final Object... params) { | ||
| try (final var connection = requireNonNull(dataSource, "dataSource가 null입니다.").getConnection(); |
There was a problem hiding this comment.
현재 "dataSource가 null입니다."라는 예외 메시지가 3군데 반복되는 것 같아요. 이경우 하나로 추출하면 추후 변경에 유리한 점이 생길 것 같아요
|
이번에 피드백 적용한 커밋까지 합쳐서 다시 커밋 링크 올리겠습니다 :) |
안녕하세요 엔델!
추석이라 좀 바빠서 구현이 늦었네요. 이번 미션동안 리뷰 잘 부탁 드립니다 :)
제가 커밋한 범위 링크입니다 => 커밋