[JDBC 라이브러리 구현하기 - 1단계] 도이(유도영) 미션 제출합니다. #316
Conversation
|
안녕하세요 도이! 연휴인데도 빠르게 하셨네요 ㅎㅎ 제가 가족들을 만나는 중이라 내일 오전 내로 리뷰 해드리겠습니다~~! |
There was a problem hiding this comment.
안녕하세요 도이~ 추석인데도 일찍 해주셨는데 늦게 리뷰 달아드려 죄송해요 ㅜ
전반적으로 잘 짜주셔서 읽고 이해하는데도 큰 문제 없이 잘 짜신것 같더라구요! 예외 처리도 잘하시구 요구사항 만족도 잘 되어있는 것 같아 부분부분 궁금한 것만 달았습니다~!
추천드리고 싶은건 ConnectionPool도 다음 스텝때 구현해보시는건 어떨까요? 학습 테스트도 있고 해서 한 번 해보시면 좋을것 같아요! 물론 요구사항은 아니였어서 추천입니다. 다음 스탭도 화이팅하시고 이번 스탭은 바로 머지하겠습니다~
| config.addDataSourceProperty("cachePrepStmts", "true"); | ||
| config.addDataSourceProperty("prepStmtCacheSize", "250"); | ||
| config.addDataSourceProperty("prepStmtCacheSqlLimit", "2048"); | ||
| config.addDataSourceProperty("useServerPrepStmts", "true"); | ||
| config.addDataSourceProperty("useLocalSessionState", "true"); | ||
| config.addDataSourceProperty("rewriteBatchedStatements", "true"); | ||
| config.addDataSourceProperty("cacheResultSetMetadata", "true"); | ||
| config.addDataSourceProperty("cacheServerConfiguration", "true"); | ||
| config.addDataSourceProperty("elideSetAutoCommits", "true"); | ||
| config.addDataSourceProperty("maintainTimeStats", "false"); |
There was a problem hiding this comment.
오 혹시 이 설정도 하신 이유나 레퍼런스가 있으실까요?! 출처 링크만 달아주셔도 좋을것 같아요!
There was a problem hiding this comment.
HikariCP 공식문서에서 추천하는 MySQL 설정값을 적용해보았습니다!
그런데 해당 테스트의 시간 측정 결과만 봤을 때에는 오차 범위가 커서 확실한 비교가 되는지는 아직 모르겠습니다 😅
https://github.com/brettwooldridge/HikariCP/wiki/MySQL-Configuration
|
|
||
| public Connection getConnection() throws CannotGetJdbcConnectionException { | ||
| try { | ||
| DataSource dataSource = DataSourceConfig.getInstance(); |
There was a problem hiding this comment.
어차피 DataSourceConfig 자체가 싱글톤이기 때문에 크게 상관은 없을것 같지만 필드로 만들어 초기화하는 것은 어떨까요? getConnection()을 할때마다 추가적으로 DataSourceConfig.getInstance()가 호출되고 결국에는 같은 인스턴스를 반환받기 때문에 불필요하다는 생각이 들었습니다!
| private static final String WRONG_QUERY_ERROR_MESSAGE_FORMAT = "SQL Query error: %s \nSQL: %s"; | ||
|
|
||
| public SqlQueryException(final String message, final String query) { | ||
| super(String.format(WRONG_QUERY_ERROR_MESSAGE_FORMAT, message, query)); |
There was a problem hiding this comment.
오옹 SqlQueryException 좋네요! 예외를 확인하기 편한 메세지 구조인 것 같습니다! 👍
|
|
||
| import java.sql.Connection; | ||
|
|
||
| public interface ConnectionManager { |
There was a problem hiding this comment.
ConnectionManager 인터페이스도 적절한 곳에 잘 두신 것 같네요! 역할 자체는 DataSource에서 Connection을 가져오는데 가져올때 에외가 발생하면 예외를 처리해주는 역할 같은데 맞을까요?!
There was a problem hiding this comment.
네 맞습니다!
그리고 덧붙이자면,
Connection을 얻는 방법으로는 DriverManager도 있기 때문에 이를 프레임워크 단의 인터페이스로 정의했습니다.
그리고 사용자가 인터페이스를 구현해 원하는 방식으로 connection을 얻어오게 하고 싶었어요.
하지만 왠만하면 DataSource를 사용할 것 같아서.. DataSoucerConnectionManager도 프레임워크에서 제공해도 되지 않나? 싶기도 하네요 😅😅
에코라면, 해당 인터페이스의 구현체는 어느 패키지에 두실 것 같나요?
| config.addDataSourceProperty("cachePrepStmts", "true"); | ||
| config.addDataSourceProperty("prepStmtCacheSize", "250"); | ||
| config.addDataSourceProperty("prepStmtCacheSqlLimit", "2048"); | ||
| config.addDataSourceProperty("useServerPrepStmts", "true"); | ||
| config.addDataSourceProperty("useLocalSessionState", "true"); | ||
| config.addDataSourceProperty("rewriteBatchedStatements", "true"); | ||
| config.addDataSourceProperty("cacheResultSetMetadata", "true"); | ||
| config.addDataSourceProperty("cacheServerConfiguration", "true"); | ||
| config.addDataSourceProperty("elideSetAutoCommits", "true"); | ||
| config.addDataSourceProperty("maintainTimeStats", "false"); |
There was a problem hiding this comment.
HikariCP 공식문서에서 추천하는 MySQL 설정값을 적용해보았습니다!
그런데 해당 테스트의 시간 측정 결과만 봤을 때에는 오차 범위가 커서 확실한 비교가 되는지는 아직 모르겠습니다 😅
https://github.com/brettwooldridge/HikariCP/wiki/MySQL-Configuration
|
|
||
| public Connection getConnection() throws CannotGetJdbcConnectionException { | ||
| try { | ||
| DataSource dataSource = DataSourceConfig.getInstance(); |
|
|
||
| import java.sql.Connection; | ||
|
|
||
| public interface ConnectionManager { |
There was a problem hiding this comment.
네 맞습니다!
그리고 덧붙이자면,
Connection을 얻는 방법으로는 DriverManager도 있기 때문에 이를 프레임워크 단의 인터페이스로 정의했습니다.
그리고 사용자가 인터페이스를 구현해 원하는 방식으로 connection을 얻어오게 하고 싶었어요.
하지만 왠만하면 DataSource를 사용할 것 같아서.. DataSoucerConnectionManager도 프레임워크에서 제공해도 되지 않나? 싶기도 하네요 😅😅
에코라면, 해당 인터페이스의 구현체는 어느 패키지에 두실 것 같나요?
안녕하세요 에코! 🚀
코드 리뷰로 만나게 되어 반갑습니다 ㅎㅎ 연휴는 잘 보내고 계신가요?
이번 단계에서는 JdbcTemplate을 통해 중복 코드를 제거하는 데에만 집중했습니다.
의문이 드는 점이 있다면 얼마든지 코멘트 또는 디엠 남겨주세요!
잘 부탁드립니다!!! 🙇♀️
optimize imports를 한 커밋이 함께 있어서..
이 커밋 범위 + 추가 커밋(조회 쿼리 단건, 다건 분리) 로 확인하시면 편하실 것 같습니다!