-
Notifications
You must be signed in to change notification settings - Fork 300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JDBC 라이브러리 구현하기 - 4단계] 매튜(김재연) 미션 제출합니다. #520
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 매! 재연씨!
지난 피드백이 꿀맛이었다니 영광입니다ㅎㅎㅎ
크게 변동 사항이 없다고 하셨지만 정말 멋지게 구현해 주셨네요! 많이 배웠습니다👍
전체적인 구조는 매우 좋지만 커넥션을 닫는 부분만 해결해 주시면 될 것 같습니다.
PreparedStatementCreator preparedStatementCreator = new PreparedStatementCreator(sql); | ||
PreparedStatementSetter preparedStatementSetter = new PreparedStatementSetter(parameters); | ||
Connection connection = DataSourceUtils.getConnection(dataSource); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TransactionExecutor를 사용하지 않고 직접 update나 다른 Jdbctemplate의 메서드를 호출하는 경우에는 Connection이 닫히지 않겠군요!
@@ -15,6 +15,8 @@ dependencies { | |||
implementation "org.apache.commons:commons-lang3:3.13.0" | |||
implementation "ch.qos.logback:logback-classic:1.2.12" | |||
|
|||
implementation "com.h2database:h2:2.2.220" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jdbc 모듈에 h2 의존성이 추가된 이유는 무엇인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DataSourceConfig 를 jdbc 모듈에 옮겨서용!
|
||
DatabasePopulatorUtils.execute(DataSourceConfig.getInstance()); | ||
final var user = new User("gugu", "password", "[email protected]"); | ||
userService.insert(user); | ||
userDao.insert(user); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
private TransactionExecutor() { | ||
} | ||
|
||
public static void transactionCommand(Runnable runnable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
최고입니다👍
|
||
public abstract class TransactionSynchronizationManager { | ||
|
||
private static final ThreadLocal<Map<DataSource, Connection>> resources = new ThreadLocal<>(); | ||
private static final ThreadLocal<Map<DataSource, Connection>> resources = ThreadLocal.withInitial(HashMap::new); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
잘 배워갑니다👍
|
||
connection.commit(); | ||
} catch (SQLException firstException) { | ||
rollback(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드로 깔끔하게 분리해 주셨군요. 멋져요.
다만, exception 변수명을 firstException, rollback 메서드 내에서는 secondException으로 사용하고 있는데요, 첫번째인지 두번째인지는 몰라도 괜찮지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모르고 안바꿨어요! 하하하하하
마! 규성씨! 제가 말씀하신 사항들을 잘 이행한지는 모르곘지만, 최대한 반영해보려 노력했습니다! 이번에도 잘 부탁드립니다! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰를 잘 반영해 주셨군요!
커넥션 닫는거 얘기했던거는 TxService를 거치지 않고 JdbcTemplate의 update를 단독으로 사용하는 경우에 커넥션이 닫히지 않아서 얘기를 했었습니다.
요구사항은 다 만족하신 것 같아서 머지하겠습니다. 위 내용만 한번 고민해주세요ㅎㅎㅎ
다음 미션도 화이팅입니다~!
떼이이이잉... 그렇군요... |
안녕하세요 마! 규성씨
저번 피드백도 너무 꿀맛이었습니다!
역시 마! 규성씨는 다르군요.
이번 PR 은 저번 PR 과 비교 하였을 때 크게 변동사항이 없는 것 같아요.
조금 있다면 DataSourceUtils 를 구현하여 사용한 것과, UserService 를 추상화한 것?
그 정도인 것 같네요 ㅎㅎ
마지막으로 이번 리뷰도 잘 부탁드린다는 말 전하면서 물러나도록 하겠습니다.
이번에도 잘 부탁드립니다 기무사 출신 스파이 마! 규성씨~!