[JDBC 라이브러리 구현하기 - 2단계] 도이(유도영) 미션 제출합니다. #421
Conversation
- 동작 자체를 테스트하는 코드이므로 예외가 터지면 테스트가 터지는 것보다는 실패 처리로 전환함
|
안녕하세요 도이~@ 내일 내로 리뷰해드리겠습니다~~! 커밋 보니까 꽤 많이 작업하신거 같은데 얼른 리뷰해드릴게요~! 고생하셨습니다! |
There was a problem hiding this comment.
안녕하세요 도이! 리뷰가 늦어져서 죄송합니다 ㅜ
이번에 CP도 적용해보셨는데 제가 무리한 요구를 드린건 아닐까 생각이 들었지만 오히려 적용해봐주시고 경험 해보셨다고 하셔서 다행이고 잘 드린것 같다는 생각이 들었네요 ㅎㅎ 감사합니다. 리팩토링 진행한 부분을 봤는데 놀랍도록 잘하셨더라구요,, 거의 교과서를 본 줄 알았습니다. 힌트도 안보고 하셨다고 하셨는데 어떤 걸 참고하셨는지도 궁금해지더라구요,, 더 얘기를 주고 받으면 좋았겠지만 3단계, 4단계도 있어서 빠르게 머지하겠습니다~~! 제가 남긴 커멘트는 다음 미션 리뷰 요청때 답 주셔도 돼요!
| TestConnectionManager connectionManager = new TestConnectionManager(); | ||
| this.jdbcTemplate = new JdbcTemplate(connectionManager); | ||
| try (Connection conn = connectionManager.getConnection()) { | ||
| conn.setAutoCommit(true); |
There was a problem hiding this comment.
혹시 AutoCommit을 true로 했다가 false로 하신 이유가 있을까요?!
There was a problem hiding this comment.
앗 사실 이 부분은 테스트 환경 설정을 빠르게 하려고
학습 테스트 PoolingVsNoPoolingTest 코드를 가져왔는데요..ㅎㅎ
DDL을 실행하는 부분만 AutoCommit으로 처리하고 나머지 테스트 내용에서는 AutoCommit으로 할 필요가 없어서(테스트 내용에 따라 직접 커밋/롤백을 하는 게 나아서) 이렇게 했다고 이해했어요.
| @@ -27,6 +27,7 @@ dependencies { | |||
| implementation "org.apache.commons:commons-lang3:3.13.0" | |||
| implementation "com.fasterxml.jackson.core:jackson-databind:2.15.2" | |||
| implementation "com.h2database:h2:2.2.220" | |||
| implementation 'com.zaxxer:HikariCP:5.0.1' | |||
There was a problem hiding this comment.
👍어떻게 보면 무리한 요구일 수 도 있었는데 직접 해보시고 좋은 경험을 하신 것 같아서 다행입니다 ㅎㅎ
| @@ -12,4 +12,8 @@ | |||
| <root level="INFO"> | |||
| <appender-ref ref="STDOUT" /> | |||
| </root> | |||
|
|
|||
| <logger name="com.zaxxer.hikari" level="DEBUG"> | |||
There was a problem hiding this comment.
Logback 설정 좋네요! 혹시 로그 설정으로 어떤 정보를 얻으셨을까요?
There was a problem hiding this comment.
해당 로그 설정으로, app 모듈의 Application을 실행했을 때 위 이미지와 같이 총 10개의 Connection이 생성되는 것을 확인할 수 있었어요. (그런데 왜 각 로그가 두번씩 출력되는지는 모르겠어요..)
그리고 주기적으로 아래와 같은 로그가 출력되는데, 계속해서 풀을 채워야 하는지 확인하는 것 같아요.
Fill pool skipped, pool has sufficient level or currently being filled (queueDepth=0).
그런데 connection을 가져가고 반납하는 내용은 해당 설정에서는 확인이 어려웠네요..!
| final PreparedStatement preparedStatement = connection.prepareStatement(query)) { | ||
| log.info("query: {}", query); | ||
| setParameters(preparedStatement, parameters); | ||
| return callback.doInConnection(connection, preparedStatement); |
There was a problem hiding this comment.
어마어마한 리팩토링이네요! 힌트를 안보고 진행하셨다고 하셨는데 그러면 직접 코드를 보신건가요? 아니면 공식문서를 보신건가요? 각 메서드별로 기능들이 확실하게 나눠져있고, 그 나눠진 로직을 람다를 이용해 깔끔하게 리팩토링하신게 인상적입니다.. 특히 callback.doInConnection은 아직도 이해를 잘 못한거 같은데 이 메서드를 통해 반환 값을 사용하는쪽에서 정의하도록 한 거죠?
There was a problem hiding this comment.
헉 감사합니다 !
이전에 JdbcTemplate가 템플릿 콜백 메서드 패턴을 사용한다는 것을 접했고, JdbcTemplate을 사용할 때 RowMapper나 ResultSetExtractor를 사용했던 경험을 기반으로 진행했습니다.
그 과정에서 JdbcTemplate이 사용하는 콜백 인터페이스 중 ConncectionCallback 인터페이스의 시그니처를 보고 제 코드에 적용시켜봤어요.
특히 callback.doInConnection은 아직도 이해를 잘 못한거 같은데 이 메서드를 통해 반환 값을 사용하는쪽에서 정의하도록 한 거죠?
네 맞습니다! 사실 헷갈리셨을 것 같은게,
제출 후 코드를 다시 보니 제가 doInConnection에서 connection, preparedStatement를 받고 있지만 사실 connection은 사용하지 않고 있더라구요. (그래서 적절한 시그니처와 클래스 네이밍은 아니었던 것 같네용..ㅎㅎ 3단계에서 수정하려고 합니다)
에코가 말씀해주신 것처럼 preparedStatement를 사용해 반환값을 정의하는 역할만 하고 있다고 보시면 될 것 같아요.
안녕하세요 에코!!
연휴에도 빠르게 리뷰해주셨는데, 프로젝트 우선순위에 밀려서 2단계를 조금 늦게 제출하네요..ㅎㅎ
지난 리뷰 #316 에 코멘트 답변들도 남겨두었습니다~!
이번 리팩터링에서는 힌트를 보지 않고 기존 JdbcTemplate의 중복 코드를 제거하는 데 집중했습니다.
이번에도 좋은 리뷰 부탁드려요 🙇♀️ 감사합니다!!