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단계] 하마드(이건회) 미션 제출합니다. #278

Merged
merged 6 commits into from
Sep 28, 2023

Conversation

rawfishthelgh
Copy link

@rawfishthelgh rawfishthelgh commented Sep 27, 2023

여우(Fox) 안녕하세요. 하마드입니다.
JDBC 라이브러리 구현하기 1단계를 "진행시켜" 봤습니다.
매서운 리뷰 부탁드립니다ㅎㅎ

충돌 해결하는 커밋이 쌓여서, 제 코드 변경 사항은
여기
서 보면 될 것 같아요

Copy link

@BackFoxx BackFoxx left a comment

Choose a reason for hiding this comment

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

주인닮아 코드가 잘생겼네요
2단계에서 뵙겠습니다

rs.getString("account"),
rs.getString("password"),
rs.getString("email")
)));
}

Choose a reason for hiding this comment

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

실제 jdbcTemplate의 API 생김새를 그대로 재현했군요
멋있다진짜

log.debug("query : {}", sql);
for (int i = 1; i < arguments.length + 1; i++) {
pstmt.setObject(i, arguments[i - 1]);
}

Choose a reason for hiding this comment

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

arguments를 배열로 바꾸어 향상된 for문을 쓸 수도 있었을 텐데 이 for문을 사용합 이유가 있읍니까?

Choose a reason for hiding this comment

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

미션을 직접 해보니 저렇게 쓸 수밖에 없네요
죄송합니다 >.0..

}


public User findById(final Long id) {
final var sql = "select id, account, password, email from users where id = ?";

Choose a reason for hiding this comment

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

이번에 리플렉션을 많이 체화하셨으니
시간이 나신다면 이참에 namedParameter를 직접 만들어보는 것도 재미있을 것입니다

@BackFoxx BackFoxx merged commit ee19a39 into woowacourse:rawfishthelgh Sep 28, 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.

3 participants