[1단계 - JDBC 라이브러리 구현하기] - 알파카(최휘용) 미션 제출합니다. #698
Conversation
There was a problem hiding this comment.
안녕하세요, 알파카! 클로버입니다.
JDBC 미션 기간동안 잘 부탁드려요 ㅎㅅㅎ
구현을 너무 잘해주셔서 리뷰를 달 곳이 많지 않았네요!
그래서 질문 위주로 리뷰를 남겼으니 알파카의 의견을 많이 공유해주시죠~
의견 공유를 위해 RC 한 번 드릴게요.
테스트 관련
이번 미션에서는 JdbcTemplate의 테스트를 짜는 부분에서 고민을 좀 했는데요.
실제로 JdbcTemplate의 동작만을 테스트할 수 있도록 Mock을 사용했는데 이 때문에 실제로 db에 잘 반영되는지는 확인하기가 어렵게 되더라구요. 이에 대한 클로버의 생각이나 클로버는 어떤 식으로 구현했는지 궁금합니다!
저도 이 부분에 대해 Mocking을 사용하는게 유의미한 테스트인지 많이 고민했었는데요, 호티와 리뷰를 진행하는 과정에서 Mocking 되는 부분은 이미 Java가 만들어 제공해주는 라이브러리이고, 직접 구현한 부분(Ex. 한 개의 결과 반환, 여러 개의 결과 반환, 예외 발생 시 처리)는 mocking 하지 않고 진행된다고 결론을 내려 mocking을 사용한 테스트를 진행했습니다.
h2를 연결하는 방법도 고려했었는데 벤더에 구애받지 않기 위해 등장한게 JDBC API인데 특정 벤더를 사용해 테스트를 진행하는 것도 맥락이 맞지 않는다 생각해 도입하지 않았습니다.
남은 기간동안 파이팅입니다.
| } | ||
|
|
||
| public void update(final User user) { | ||
| // todo | ||
| final var sql = "update users set account = ?, password = ?, email = ?"; |
There was a problem hiding this comment.
질문)
var 타입을 사용한 타입 추론을 사용하셨네요. 타입 추론을 사용하게 된 특별한 이유가 있나요?
다른 변수는 모두 타입 지정이 되었는데, sql문만 var 타입이라 궁금해요!
There was a problem hiding this comment.
안녕하세요 특별한 이유는 없고 이미 코드에 존재하던 sql 변수들을 끌어와 내용만 바꾸는 식으로 구현했어서 놓친 부분입니다 ㅎㅎ 체크를 잘 못했네요..
통일성을 위해서 다른 변수들과 마찬가지로 타입 추론을 하지 않도록 수정했습니다!
final도 마찬가지로 붙게 됐는데 이것도 제거해주겠습니다짐육
꼼꼼한 리뷰 감사합니다~ 🙇
|
|
||
| private <T> List<T> mapResultSetToList(RowMapper<T> rowMapper, ResultSet rs) throws SQLException { | ||
| List<T> result = new ArrayList<>(); | ||
| int rowNum = 0; |
There was a problem hiding this comment.
질문)
rowNum은 어떤 역할을 하는 인자길래 모든 결과에 대해 0을 넣어주게 구현이 되었나요?
알파카가 생각하는 rowNum의 의미가 궁금해요~
There was a problem hiding this comment.
헉 일단 동작 확인을 위해서 값을 임시로 넣어둔 거였는데 수정하지 못했네요... 찾아주셔서 감사합니다 🙇
원래의 의미대로 현재 ResultSet의 행 번호를 사용하도록 수정했습니다!!
|
|
||
| return mapResultSetToList(rowMapper, rs); | ||
| } catch (SQLException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
RuntimeException말고 좀 더 유의미한 예외를 던져보면 좋을 것 같아요!
There was a problem hiding this comment.
DataAccessException을 활용하도록 수정했습니다!
| public void query() throws Exception { | ||
| when(preparedStatement.executeQuery()).thenReturn(resultSet); | ||
| when(resultSet.next()).thenReturn(true, true, true, false); | ||
| when(resultSet.getString("name")).thenReturn("이은정", "클로버", "지니아"); |
| @@ -45,8 +45,8 @@ void testRequired() { | |||
|
|
|||
| log.info("transactions : {}", actual); | |||
| assertThat(actual) | |||
| .hasSize(0) | |||
| .containsExactly(""); | |||
| .hasSize(1) | |||
There was a problem hiding this comment.
주석에 달린 왜 그런 결과가 나왔을까? 이런 질문들에도 답변 달아주시면 좋을 것 같습니다!
다음 PR에 포함해주세요 😄
안녕하세요 클로버~
이렇게 미션에서 만나니 감회가 새롭네요 ㅎㅎ
부담없이 남겨주시면 열심히 반영하고 의견 남겨보겠습니다.
이번 미션에서는
JdbcTemplate의 테스트를 짜는 부분에서 고민을 좀 했는데요.실제로
JdbcTemplate의 동작만을 테스트할 수 있도록 Mock을 사용했는데 이 때문에 실제로 db에 잘 반영되는지는 확인하기가 어렵게 되더라구요. 이에 대한 클로버의 생각이나 클로버는 어떤 식으로 구현했는지 궁금합니다!잘 부탁드립니다.