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단계] 로이스(원태연) 미션 제출합니다. #277

Merged
merged 12 commits into from
Sep 29, 2023

Conversation

TaeyeonRoyce
Copy link
Member

@TaeyeonRoyce TaeyeonRoyce commented Sep 27, 2023

안녕하세요, 아벨. 페어 이후 오랜만에 뵙네요!
JdbcTemplate을 통해 UserDao에서 중복되는 코드를 옮겨보았습니다.

이번 리뷰 요청 범위입니다! 커밋s
리뷰 잘 부탁드려요!

Copy link

@tjdtls690 tjdtls690 left a comment

Choose a reason for hiding this comment

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

안녕하세요 로이스!

1단계는 그냥 깔끔하게 잘 구현해 주셔서 피드백 드릴게 없네요. 몇 가지 코멘트 남겼는데 일단 머지 하겠습니다. 이후에 2단계 시작하면서 답변 달아주시면 될 것 같습니다 :)

}
}

private void setAllArguments(final Object[] args, final PreparedStatement preparedStatement) throws SQLException {

Choose a reason for hiding this comment

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

전 개인적으로 여러 메서드에서 공용으로 쓰이는 setAllArguments 메서드 같은 경우는 가장 아래쪽에 메서드를 배치하는 편인데, 로이스는 처음으로 호출된 메서드 바로 밑에 두시나보군요. 어떤 기준이 있는 것인지 궁금하네요!

}

public List<User> findAll() {
// todo
return null;
final var sql = "select id, account, password, email from users";

Choose a reason for hiding this comment

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

var를 쓰시면서 로이스가 체감한 장단점이 있다면 어떤 것들이 있을 지 궁금합니다 :)

@tjdtls690 tjdtls690 merged commit 0d41295 into woowacourse:taeyeonroyce Sep 29, 2023
1 check failed
TaeyeonRoyce added a commit to TaeyeonRoyce/jwp-dashboard-jdbc that referenced this pull request Oct 2, 2023
* 패키지 위치 변경 및 코드 정리

* 패키지 위치 변경 및 코드 정리

* 패키지 위치 변경 및 코드 정리

* feat: JdbcTemplate update 메서드 추가

* test: UserHistory log 테스트 추가

* refactor: 이전 버전 Insert 제거

* feat: RowMapper 추가

* feat: 단일 객체 조회(queryForObject) 추가

* feat: findByAccount 추가

* feat: findAll 추가

* feat: update 추가

* style: 불필요한 import 제거

---------

Co-authored-by: kang-hyungu <[email protected]>
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