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 라이브러리 구현하기 - 1단계] 로지(윤가영) 미션 제출합니다. #281

Merged
merged 3 commits into from
Sep 30, 2023

Conversation

kyY00n
Copy link
Member

@kyY00n kyY00n commented Sep 27, 2023

No description provided.

@iamjooon2 iamjooon2 self-requested a review September 27, 2023 06:51
Copy link
Member

@iamjooon2 iamjooon2 left a comment

Choose a reason for hiding this comment

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

안녕하세요 로지🫡
몸은 좀 괜찮으신가요?
1단계 커밋 범위만 지정해주신 덕에 편하게 리뷰했습니다
제 미션 부랴부랴 끝내고 빠르게 남겨보았습니다!

Comment on lines 57 to 64
List<T> query = query(sql, rowMapper, params);
if (query.size() > 1) {
throw new RuntimeException("too many result");
}
if (query.isEmpty()) {
return null;
}
return query.get(0);
Copy link
Member

Choose a reason for hiding this comment

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

쿼리 재사용 👍

Comment on lines 81 to 85
final List<T> results = new ArrayList<>();
if (rs.next()) {
results.add(rowMapper.mapRow(rs, 0));
}
return results;
Copy link
Member

Choose a reason for hiding this comment

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

resultSet 내의 row가 여러개일 때는 어떻게 되나요?🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

그러네요...

Copy link
Member

@iamjooon2 iamjooon2 left a comment

Choose a reason for hiding this comment

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

요구사항을 잘 충족하셔서 1단계는 머지하겠습니다
몸보다 마음이 풍요로운 한가위 되시길 바랍니다

Comment on lines +25 to +26
try (final Connection conn = dataSource.getConnection();
final PreparedStatement pstmt = conn.prepareStatement(sql)) {
Copy link
Member

Choose a reason for hiding this comment

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

try-with-resource 도 적용해주셨군요!👍

throw new RuntimeException("too many result");
List<T> results = query(sql, rowMapper, params);
if (results.size() > 1) {
throw new RuntimeException("too many result. expected 1 but was " + results.size());
Copy link
Member

Choose a reason for hiding this comment

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

예외처리 꼼꼼하게 하시네요👍

}
return query.get(0);
return results.get(0);
}
Copy link
Member

Choose a reason for hiding this comment

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

변수명도 변경해주셨군요

@iamjooon2 iamjooon2 merged commit 61c94c3 into woowacourse:kyy00n Sep 30, 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