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단계] 애쉬(정설희) 미션 제출합니다 #319

Merged
merged 5 commits into from
Sep 29, 2023

Conversation

xxeol2
Copy link

@xxeol2 xxeol2 commented Sep 29, 2023

안녕하세요 로건! 😎
발표 스터디 이후로 오랜만이네요 ㅎㅎ 즐거운 명절 보내고 계신가요?! 🌕

요구사항을 만족시키는 선에서 간단하게 구현해봤어요!
queryForObject와 query 메서드에서 중복이 있는 것 같아 아쉬움이 있는데,
2단계 요구사항이 리팩토링인 만큼 우선 돌아가는 코드를 작성하는데 집중했어요!

그럼 리뷰 잘 부탁드립니다 🙇🏻‍♀️

@xxeol2 xxeol2 changed the base branch from main to xxeol2 September 29, 2023 06:35
Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

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

안녕하세요 애쉬 ~ 완전 오랜만이네요!

저는 추석 덕분에 편하게 잘 쉬고 있습니다 ㅋㅋ
🎪 페스타고 동시성 글 올리신 거 덕분에 많이 배워 갔어요~ 👍👍
이번 단계에서는 애쉬의 생각이 궁금한 부분 코멘트 남겼어요! 이 부분은 2단계 진행하면서 올려주시면 될 것 같아요

다음 미션도 화이팅하시고 추석 잘 보내세요~!! 🌕︎

Comment on lines 24 to 26
public UserDao(final DataSource dataSource) {
this.dataSource = dataSource;
this.jdbcTemplate = new JdbcTemplate(dataSource);
}
Copy link
Member

Choose a reason for hiding this comment

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

저는 이 부분을 삭제했는데요. 왜냐하면 앞으로 JdbcTemplate으로 다 대체가 될 것 같아보이기도 하고, 일단 삭제한다음 다음에 필요한 상황이 있으면 그때 생성자를 다시 추가해도 늦지 않을 것 같더라구요. 그래서 UserDaoTestJdbcTemplate로 바꾸게 되었어요.
그런 의미로 애쉬는 이 부분을 남겨두신 이유가 궁금해요~

Copy link
Author

Choose a reason for hiding this comment

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

UserDao 내부 로직만 변경하고, UserDao를 사용하는 측의 코드는 변경이 없도록 하기 위해서 기존 생성자를 놔두었습니다 !!

import java.sql.ResultSet;
import java.sql.SQLException;

@FunctionalInterface
Copy link
Member

Choose a reason for hiding this comment

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

👍👍

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.sql.DataSource;
import org.springframework.dao.EmptyResultDataAccessException;

public class JdbcTemplate {
Copy link
Member

Choose a reason for hiding this comment

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

리뷰하면서 코드를 읽는데, 자꾸 어떤걸 놓친 기분이 들어서 여러번 읽어보다가 특별한 점을 하나 발견했는데요 🧐
UserDao에는 전부 final이 붙어 있지만, JdbcTemplate에서는 final이 없더라구요
이 부분도 통일성 있게 코드를 작성하신 것 같아서 혹시 따로 규칙이 있는지 궁금해요 !

Copy link
Author

Choose a reason for hiding this comment

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

프로젝트에서 컨벤션이 final이 아니어서, IntelliJ 설정상 변수 추출시 final이 붙지않았네요 ㅠ
그래서 구구가 작성해주신 기존 코드는 final이 붙고 제 코드는 final이 안붙은 것 같아요!!
이 부분 일관성있게 수정할게요!

@70825 70825 merged commit 49fb842 into woowacourse:xxeol2 Sep 29, 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