Skip to content
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단계] 홍고(홍여진) 미션 제출합니다. #604

Merged
merged 7 commits into from
Oct 11, 2023

Conversation

hgo641
Copy link

@hgo641 hgo641 commented Oct 10, 2023

안녕하세요, 깃짱!
마지막 4단계입니다ㅠㅠ
이번에도 리뷰 잘 부탁드립니다! 🙇‍♂️🙇‍♀️

@hgo641 hgo641 requested a review from gitchannn October 10, 2023 13:51
@hgo641 hgo641 self-assigned this Oct 10, 2023
Copy link

@gitchannn gitchannn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

홍고!
구현한 내용 잘 확인했습니다!
코드상으로도 문제가 크지 않은 것 같고, 간단한 코멘트 하나 남겨놨어요!
새로운 미션 해야 하니깐 이만 머지할게요 수고했어요 🌟


@Override
public User findById(long id) {
return transactionManager.execute(() -> userService.findById(id));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깔끔하게 잘 구현했네요! ㅎㅎ

public class TxUserService implements UserService {

private final UserService userService;
private final TransactionManager transactionManager;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

트랜잭션 매니저를 사용해 구현하셨군요!

final PreparedStatement preparedStatement = connection.prepareStatement(sql)
final Connection connection = DataSourceUtils.getConnection(dataSource);
try (
final PreparedStatement preparedStatement = connection.prepareStatement(sql)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

트랜잭션을 통하지 않고 JdbcTemplate 코드를 직접 실행하는 경우에 커넥션이 닫히지 않는 문제가 발생할 것 같은데요!
이거에 대한 홍고의 생각이 궁금합니다!

@gitchannn gitchannn merged commit 796f8a2 into woowacourse:hgo641 Oct 11, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants