[3단계 - Transaction 적용하기] 멍구(이유영) 미션 제출합니다. #1241
Conversation
WalkthroughJDBC 호출이 명시적 Connection 전달 방식으로 전환되었습니다. DAO(UserDao, UserHistoryDao)와 쿼리 빌더(Select/Insert/Update/Delete)의 실행 메서드 시그니처들이 모두 java.sql.Connection 파라미터를 받도록 변경되었고, JdbcTemplate에는 Connection을 인수로 받는 오버로드들이 추가되었습니다. TransactionTemplate과 TransactionalWork가 새로 도입되어 UserService는 TransactionTemplate을 주입받아 트랜잭션 경계 안에서 제공된 Connection으로 DAO 호출을 수행하도록 변경되었으며, 테스트와 목 객체들도 이에 맞춰 수정되었습니다. 개요
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (1)
54-67: 중복된 update 로직을 재사용해볼까요? Line 54-67에서 기존update(String, Object...)와 거의 동일한 흐름이 한 번 더 등장합니다. 동일한 파라미터 바인딩/예외 처리 코드가 두 곳에 퍼져 있으면 추후 변경 시 놓치기 쉽고, 테스트 부담도 커져요. 공통 부분을 묶어두면 연결 방식이 늘어나도 한 곳만 관리하면 되니 유지보수성이 높아질 텐데, 어떤 형태의 추출이 가능할지 한 번 고민해보시면 어떨까요?app/src/main/java/com/techcourse/dao/UserDao.java (1)
33-36: 두 가지 update 경로의 일관성을 어떻게 맞출까요? Line 33-36에서 새로 추가된 메서드는 직접 SQL 문자열을 작성하고, 기존 메서드는 쿼리 빌더 체인을 사용하고 있네요. 같은 책임이 서로 다른 표현식으로 분리되면 향후 컬럼이 추가·수정될 때 한쪽만 바뀌는 위험이 커집니다. 커넥션을 주입받는 상황에서도 동일한 빌더를 활용하거나, 공통 쿼리 정의를 공유하는 구조를 만들어볼 수 있을까요?app/src/test/java/com/techcourse/service/UserServiceTest.java (1)
26-31: 이미 확보한 DataSource를 적극 활용해볼까요? Line 26-31과 이후 UserService 생성 구간에서DataSourceConfig.getInstance()를 반복 호출하고 있습니다. 한 번 주입해 둔dataSource필드를 그대로 활용하면 의도가 더 분명해지고, 테스트 설정도 단일 책임으로 유지할 수 있지 않을까요?Also applies to: 38-54
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/techcourse/dao/UserDao.java(2 hunks)app/src/main/java/com/techcourse/dao/UserHistoryDao.java(2 hunks)app/src/main/java/com/techcourse/service/UserService.java(2 hunks)app/src/test/java/com/techcourse/service/MockUserHistoryDao.java(2 hunks)app/src/test/java/com/techcourse/service/UserServiceTest.java(3 hunks)jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.📚 학습 원칙
- 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
- 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
- 작은 단계로 문제를 분해하여 접근하도록 도와주세요
💡 피드백 방법
- 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
- 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
- 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
- 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록
⚡ 객체지향 생활 체조 원칙 검토
다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만
- 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
- 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
- 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"
규칙 2: else 예약어 금지
- else, switch/case 사용 금지
- 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
- 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"
규칙 3: 모든 원시값과 문자열 포장
- 원시 타입과 String을 객체로 포장
- 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
- 💡 예시:
int port→Port port,String name→Name name규칙 4: 한 줄에 점 하나만 (디미터 법칙)
- 메서드 체이닝 제한
- 📖 이유: 결합도 감소, 캡슐화 향상
- 💡 나쁜 예:
request.getUri().getPath().substring()- 💡 좋은 예:
request.extractPath()규칙 5: 축약 금지
- 명확한 이름 사용 (축약어 금지)
- 📖 이유: 코드 가독성, 의도 명확화
- 💡 예시:
req→request,calcAmt→calculateAmount규칙 6: 모든 엔티티를 작게 유지
- 클래스 50줄, 메서드 10줄 이하
- 📖 이유: 단일 책임, 이해와 테스트 용이성
- 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"
규칙 7: 인스턴스 변수 3개 이하
- 클래스당 최대 3개의 인스턴스 변수
- 📖 이유: 높은 응집도, 단일 책임 유지
- 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"
규칙 8: 일급 컬렉션 사용
- 컬렉션을 감싸는 클래스 사용
- 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
- 💡 예시:
List<HttpHeader>→HttpHeaders클래스규칙 9: 게터/세터/프로퍼티 금지
- Tell, Don't Ask 원칙 준수
- 📖 이유: 진정한 캡슐화, 객체 자율성
- 💡 나쁜 예:
if (user.getPassword().equals(input))- 💡 좋은 예:
if (user.authenticate(input))🎯 추가 프로그래밍 요구사항
코드 스타일
- Java Style Guide 준수
- 3항 연산자 사용 금지
테스트 및 구조
- **모든 기능 T...
Files:
app/src/main/java/com/techcourse/dao/UserHistoryDao.javaapp/src/main/java/com/techcourse/service/UserService.javaapp/src/test/java/com/techcourse/service/MockUserHistoryDao.javaapp/src/test/java/com/techcourse/service/UserServiceTest.javajdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.javaapp/src/main/java/com/techcourse/dao/UserDao.java
🧬 Code graph analysis (2)
app/src/main/java/com/techcourse/service/UserService.java (3)
app/src/main/java/com/techcourse/dao/UserDao.java (1)
UserDao(9-60)app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
UserHistoryDao(9-41)app/src/main/java/com/techcourse/domain/UserHistory.java (1)
UserHistory(5-59)
app/src/test/java/com/techcourse/service/UserServiceTest.java (1)
app/src/main/java/com/techcourse/config/DataSourceConfig.java (1)
DataSourceConfig(7-27)
🔇 Additional comments (1)
app/src/test/java/com/techcourse/service/MockUserHistoryDao.java (1)
16-17: 모든 log 호출 경로가 동일하게 동작할까요? Line 16-17에서 커넥션 버전만 예외를 던지도록 바뀌면서, 기존log(UserHistory)호출은 더 이상 Mock에서 차단되지 않습니다. 테스트나 다른 코드가 여전히 파라미터 하나짜리 버전을 기대한다면, 의도치 않게 실제 DB 작업이 실행될 수도 있어요. 두 시그니처가 동일한 Mock 행위를 보장하도록 재점검해보면 어떨까요?
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectWhereStep.java (1)
11-13: Connection을 명시적으로 전달하는 설계 선택에 대해 생각해보셨나요?인터페이스의 모든 조회 메서드가 이제
Connection을 파라미터로 받도록 변경되었네요. 이는 트랜잭션 경계를 호출자가 명시적으로 제어할 수 있게 하는 장점이 있습니다.다만 몇 가지 고려해볼 점이 있어요:
- 이 방식을 선택하면 Connection의 생명주기 관리 책임이 누구에게 있나요?
- Connection을 받는 메서드와 받지 않는 메서드 두 가지를 모두 제공하는 방식과 비교했을 때, 현재 선택의 장단점은 무엇일까요?
- 트랜잭션이 필요 없는 단순 조회의 경우에도 매번 Connection을 전달해야 하는데, 이것이 사용성에 미치는 영향은 어떨까요?
jdbc/src/main/java/com/interface21/transaction/support/TransactionalWork.java (1)
5-8: 과도하게 넓은 예외 선언
execute메서드가throws Exception으로 선언되어 있습니다. 이는 매우 광범위한 예외 처리 방식입니다.고민해볼 점:
- 모든 종류의 예외를 던질 수 있게 하는 것이 좋은 설계일까요?
- 호출하는 쪽(TransactionTemplate)에서 어떤 예외가 발생할지 예측할 수 있을까요?
- 체크 예외와 언체크 예외를 구분하지 않는 것이 적절한가요?
힌트: 다른 템플릿 패턴 구현체들(예: Spring의 TransactionCallback)은 어떤 방식을 사용하는지 살펴보시면 도움이 될 것 같습니다.
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectQueryBuilder.java (1)
12-17: 인스턴스 변수가 많아 클래스의 책임이 분산되어 있습니다객체지향 생활 체조 규칙 7번(인스턴스 변수 3개 이하)을 고려해볼 필요가 있습니다. 현재 클래스는 6개의 인스턴스 변수를 가지고 있습니다.
생각해볼 점:
selectedColumns,whereColumns,whereParams는 각각 쿼리의 다른 부분을 담당하고 있습니다. 이들을 하나의 객체로 묶을 수 있을까요?- 예를 들어,
WhereClause같은 객체를 만들어 where 관련 컬럼과 파라미터를 함께 관리하면 어떨까요?📖 이유: 관련된 데이터를 함께 묶으면 응집도가 높아지고, 각 객체의 책임이 명확해집니다.
app/src/main/java/com/techcourse/service/UserService.java (1)
24-31: 서비스 계층에서 Connection을 드러내고 있어요public 메서드가
Connection을 그대로 요구하게 되면서 서비스 레이어가 JDBC 구현 세부사항에 묶이고, 컨트롤러나 다른 도메인 협력자가 인프라 자원 수명까지 직접 관리해야 하는 구조가 되었습니다. 트랜잭션 경계 설정 역할을 서비스 내부로 되돌리거나, 조회용 템플릿이 결과를 반환할 수 있도록 확장해 보면 이런 결합을 줄일 길이 보이지 않을까요? As per coding guidelinesjdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (2)
38-51: update 메서드가 객체지향 생활 체조 규칙을 어기고 있어요try-with-resources 안에서 for 루프까지 겹치면서 들여쓰기 깊이가 2단계를 넘어가고, 전체 라인 수도 10라인을 훌쩍 넘네요. 파라미터 바인딩이나 예외 변환 책임을 별도 협력 객체나 메서드로 떼어낸다면 depth도 줄이고 SRP도 지킬 수 있을 것 같은데, 어떤 분리가 가능할지 한번 실험해 보실래요? As per coding guidelines
70-89: query 메서드도 한 단계 들여쓰기 원칙을 지키기 어렵네요중첩된 try-with-resources와 while 루프 때문에 깊이가 3단계까지 내려가고, 라인 수 역시 규칙을 초과하고 있습니다. 결과 매핑과 자원 관리 로직을 각각의 협력자로 위임하거나, 전용
ResultSet추상화를 도입하면 가독성과 응집도를 동시에 높일 수 있지 않을까요? As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/src/main/java/com/techcourse/dao/UserDao.java(2 hunks)app/src/main/java/com/techcourse/dao/UserHistoryDao.java(2 hunks)app/src/main/java/com/techcourse/service/UserService.java(1 hunks)app/src/test/java/com/techcourse/dao/UserDaoTest.java(5 hunks)app/src/test/java/com/techcourse/service/UserServiceTest.java(1 hunks)jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java(4 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/delete/DeleteExecutableStep.java(1 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/delete/DeleteQueryBuilder.java(3 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/insert/InsertExecutableStep.java(1 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/insert/InsertQueryBuilder.java(3 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectExecutableStep.java(1 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectQueryBuilder.java(2 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectStringQueryBuilder.java(2 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectWhereStep.java(1 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/update/UpdateExecutableStep.java(1 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/update/UpdateQueryBuilder.java(3 hunks)jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java(1 hunks)jdbc/src/main/java/com/interface21/transaction/support/TransactionalWork.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.📚 학습 원칙
- 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
- 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
- 작은 단계로 문제를 분해하여 접근하도록 도와주세요
💡 피드백 방법
- 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
- 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
- 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
- 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록
⚡ 객체지향 생활 체조 원칙 검토
다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만
- 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
- 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
- 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"
규칙 2: else 예약어 금지
- else, switch/case 사용 금지
- 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
- 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"
규칙 3: 모든 원시값과 문자열 포장
- 원시 타입과 String을 객체로 포장
- 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
- 💡 예시:
int port→Port port,String name→Name name규칙 4: 한 줄에 점 하나만 (디미터 법칙)
- 메서드 체이닝 제한
- 📖 이유: 결합도 감소, 캡슐화 향상
- 💡 나쁜 예:
request.getUri().getPath().substring()- 💡 좋은 예:
request.extractPath()규칙 5: 축약 금지
- 명확한 이름 사용 (축약어 금지)
- 📖 이유: 코드 가독성, 의도 명확화
- 💡 예시:
req→request,calcAmt→calculateAmount규칙 6: 모든 엔티티를 작게 유지
- 클래스 50줄, 메서드 10줄 이하
- 📖 이유: 단일 책임, 이해와 테스트 용이성
- 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"
규칙 7: 인스턴스 변수 3개 이하
- 클래스당 최대 3개의 인스턴스 변수
- 📖 이유: 높은 응집도, 단일 책임 유지
- 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"
규칙 8: 일급 컬렉션 사용
- 컬렉션을 감싸는 클래스 사용
- 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
- 💡 예시:
List<HttpHeader>→HttpHeaders클래스규칙 9: 게터/세터/프로퍼티 금지
- Tell, Don't Ask 원칙 준수
- 📖 이유: 진정한 캡슐화, 객체 자율성
- 💡 나쁜 예:
if (user.getPassword().equals(input))- 💡 좋은 예:
if (user.authenticate(input))🎯 추가 프로그래밍 요구사항
코드 스타일
- Java Style Guide 준수
- 3항 연산자 사용 금지
테스트 및 구조
- **모든 기능 T...
Files:
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectWhereStep.javajdbc/src/main/java/com/interface21/transaction/support/TransactionalWork.javaapp/src/main/java/com/techcourse/dao/UserHistoryDao.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/delete/DeleteQueryBuilder.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectQueryBuilder.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectStringQueryBuilder.javaapp/src/main/java/com/techcourse/dao/UserDao.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectExecutableStep.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/delete/DeleteExecutableStep.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/update/UpdateExecutableStep.javajdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.javajdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.javaapp/src/test/java/com/techcourse/service/UserServiceTest.javaapp/src/main/java/com/techcourse/service/UserService.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/update/UpdateQueryBuilder.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/insert/InsertQueryBuilder.javaapp/src/test/java/com/techcourse/dao/UserDaoTest.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/insert/InsertExecutableStep.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: egaeng09
PR: woowacourse/java-jdbc#1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.
🧬 Code graph analysis (3)
app/src/test/java/com/techcourse/service/UserServiceTest.java (4)
jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java (1)
TransactionTemplate(7-30)app/src/main/java/com/techcourse/config/DataSourceConfig.java (1)
DataSourceConfig(7-27)app/src/main/java/com/techcourse/dao/UserDao.java (1)
UserDao(9-55)app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
UserHistoryDao(9-29)
app/src/main/java/com/techcourse/service/UserService.java (4)
jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java (1)
TransactionTemplate(7-30)app/src/main/java/com/techcourse/dao/UserDao.java (1)
UserDao(9-55)app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
UserHistoryDao(9-29)app/src/main/java/com/techcourse/domain/UserHistory.java (1)
UserHistory(5-59)
app/src/test/java/com/techcourse/dao/UserDaoTest.java (1)
app/src/test/java/com/techcourse/dao/TestDataSourceConfig.java (1)
TestDataSourceConfig(6-14)
🔇 Additional comments (11)
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/update/UpdateQueryBuilder.java (1)
58-82: 구현이 일관성 있게 변경되었습니다.
execute()메서드가 전달받은Connection을jdbcTemplate.update()까지 올바르게 전파하고 있네요. 인터페이스 변경과 일치하는 깔끔한 구현입니다.jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/insert/InsertQueryBuilder.java (1)
48-66: INSERT 빌더도 동일한 패턴으로 일관성 있게 수정되었습니다.Update 빌더와 마찬가지로
Connection을 올바르게 전달하고 있어요. 전체 쿼리 빌더 API의 일관성이 유지되고 있습니다.jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/insert/InsertExecutableStep.java (1)
7-7: 인터페이스 변경이 일관성 있게 적용되었습니다.jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/update/UpdateExecutableStep.java (1)
8-8: 인터페이스 변경이 일관성 있게 적용되었습니다.jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/delete/DeleteExecutableStep.java (1)
8-8: 인터페이스 변경이 일관성 있게 적용되었습니다.app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
19-27: 히스토리 로깅이 올바르게 INSERT로 구현되어 있습니다.
log()메서드가insertInto("user_history")를 사용하여 새로운 이력 레코드를 추가하고 있네요. 이전 리뷰 코멘트에서 언급된 UPDATE 관련 이슈는 현재 코드에서는 보이지 않습니다.몇 가지 확인 포인트:
insertInto()빌더가 최종적으로execute(connection)을 호출하면서 제공된 Connection을 사용하고 있어요.- UserHistory의 모든 필드(user_id, account, password, email, created_at, created_by)가 빠짐없이 매핑되고 있습니다.
- Connection이 DAO 메서드로 전달되어 트랜잭션 경계 내에서 실행될 수 있도록 구조가 변경되었네요.
구현이 깔끔하고 의도가 명확합니다!
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectStringQueryBuilder.java (1)
43-48: Connection 전달 방식에 대한 설계 고민현재
toList와findFirst메서드가 Connection을 파라미터로 받아서 JdbcTemplate에 전달하는 방식으로 구현되어 있습니다.이 설계에 대해 고민해볼 점들이 있습니다:
- 쿼리 빌더가 Connection을 파라미터로 받는 것이 적절한가요?
- Connection의 생명주기 관리 책임이 호출자에게 있는데, 이것이 명확하게 드러나고 있나요?
- 만약 Connection을 닫지 않으면 어떤 문제가 발생할까요?
다음 질문을 고민해보시면 좋을 것 같습니다:
- 현재 설계에서 Connection을 누가 열고 닫아야 할까요?
- 여러 쿼리를 하나의 트랜잭션으로 묶으려면 어떻게 해야 할까요?
- 빌더 패턴을 사용하는 이유와 Connection 전달 방식이 조화를 이루고 있나요?
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectExecutableStep.java (1)
7-11: 명확한 인터페이스 설계인터페이스가 단순하고 명확하게 정의되어 있습니다. Connection을 명시적으로 받도록 하여 실행 시점의 트랜잭션 컨텍스트를 제어할 수 있게 했네요.
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/delete/DeleteQueryBuilder.java (1)
44-46: 안전장치가 잘 구현되어 있습니다WHERE 절 없는 DELETE를 방지하는 안전장치가 있네요. 이는 실수로 인한 전체 데이터 삭제를 막아주는 좋은 방어 코드입니다.
💡 추가로 고민해볼 점: 만약 정말로 전체 데이터를 삭제해야 하는 경우가 있다면 어떻게 처리할 수 있을까요?
jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java (1)
15-29: 트랜잭션 관리의 핵심 패턴이 구현되어 있습니다전체적인 트랜잭션 관리 흐름이 잘 구현되어 있습니다:
- try-with-resources로 Connection 자동 종료
- autoCommit을 false로 설정하여 명시적 트랜잭션 제어
- 성공 시 commit, 실패 시 rollback
다만, 실무에서 고려할 수 있는 추가 개선사항들이 있습니다:
- 롤백 실패 처리: 만약
rollback()에서도 예외가 발생하면 어떻게 될까요?- 로깅: 트랜잭션 시작/커밋/롤백 시점을 로깅하면 디버깅에 도움이 될 것 같은데, 어떻게 생각하시나요?
- 트랜잭션 격리 수준: 현재는 기본 격리 수준을 사용하는데, 격리 수준을 설정할 필요가 있는 경우는 언제일까요?
이러한 부분들은 현재 미션의 범위를 넘어설 수 있지만, 나중에 고민해보시면 좋을 주제들입니다.
app/src/main/java/com/techcourse/dao/UserDao.java (1)
17-54: DAO 레이어에 Connection이 노출된 설계에 대한 고민모든 DAO 메서드가 Connection을 파라미터로 받도록 변경되었습니다. 이는 트랜잭션 관리를 위한 선택이지만, 설계상 트레이드오프가 있습니다.
생각해볼 점들:
- 추상화 수준: DAO의 클라이언트(Service 레이어)가 JDBC의 Connection을 알아야 하는 것이 적절한가요?
- 책임의 위치: Connection을 누가 생성하고, 누가 닫아야 할까요? 현재 설계에서 이 책임이 명확하게 드러나나요?
- 대안 고려: ThreadLocal을 사용하거나, TransactionTemplate 내부에서만 Connection을 관리하는 방법도 있을 텐데, 왜 이 방식을 선택하셨나요?
💡 이 설계는 "명시적 연결 전달(Explicit Connection Passing)" 패턴입니다. 장점은 트랜잭션 범위가 명확하고, 단점은 모든 레이어가 Connection을 알아야 한다는 것입니다.
이번 구현을 통해 어떤 점을 배우셨는지, 그리고 이 방식의 장단점에 대해 어떻게 생각하시는지 궁금합니다!
| public void log(final Connection connection, final UserHistory userHistory) { | ||
| jdbcTemplate.insertInto("user_history") | ||
| .value("user_id", userHistory.getUserId()) | ||
| .value("account", userHistory.getAccount()) | ||
| .value("password", userHistory.getPassword()) | ||
| .value("email", userHistory.getEmail()) | ||
| .value("created_at", userHistory.getCreatedAt()) | ||
| .value("created_by", userHistory.getCreateBy()) | ||
| .execute(connection); | ||
| } |
There was a problem hiding this comment.
이번 미션은 JdbcTemplate에 집중하는 거니 어쩔 수 없지만..
필드목록을 하나하나 전부 매핑해줘야 하는게 아쉽긴 하네요 😢
There was a problem hiding this comment.
맞습니다.. 그러나 어떤 필드가 DB에서 어떤 컬럼명으로 되어있는지는 개발자가 직접 지정해줘야하기 때문에 현재는 이게 최선인 것 같아요.
JPA처럼 @column("user_id") 처럼 엔티티 필드에 정의해두고, 리플렉션으로 읽어서 자동으로 insert문을 만들어주는 아이디어가 있겠지만 이번 미션에서는 구현하지 않을게요! 모코는 이번 미션에서 이런 문제를 해결하기 위해 어떤 시도들을 하셨나요?
There was a problem hiding this comment.
저는 말씀해주신 것처럼 어노테이션 기반 리플렉션을 통한 필드 자동 매핑을 구현했습니다.
미션 목표는 JdbcTemplate 구현이었지만 저는 Jpa 구현에 초점을 두고 진행했던 것 같아요.
그게 더 재밌어서요!! 😆
| public User findById(final long id) { | ||
| return userDao.findById(id) | ||
| public User getById(final Connection connection, final long id) { | ||
| return userDao.findById(connection, id) | ||
| .orElseThrow(() -> new IllegalArgumentException("해당 id의 user를 찾을 수 없습니다, id: " + id)); | ||
| } | ||
|
|
||
| public void insert(final User user) { | ||
| userDao.insert(user); | ||
| public void insert(final Connection connection, final User user) { | ||
| userDao.insert(connection, user); | ||
| } |
There was a problem hiding this comment.
여기에 트랜잭션을 적용하지 않은 건 불필요하다고 생각되어서인가요??
There was a problem hiding this comment.
DB 관점에서 단일 SELECT나 INSERT는 트랜잭션 관리가 필요하지 않습니다.
그러나 모코 코멘트를 보고 다시 생각해봤는데, 그러나 UserService는 서비스 계층으로서 외부에 insert와 getById를 공개하고 있으며, 이 계층은 트랜잭션 단위의 비즈니스 로직을 관리하는 책임을 가집니다.
-
따라서 insert도 사실 signup()이라고 생각한다면 트랜잭션으로 관리될 필요가 있는 비즈니스 로직이라고 생각하여 트랜잭션을 적용했습니다.
-
반면 getById는 비즈니스 로직이긴 하지만 조회 전용이므로 트랜잭션 관리가 불필요하며,
Connection 관리 책임을 JdbcTemplate에 위임해도 무방하다고 생각하여 관련해서도 코드 수정했습니다!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/src/test/java/com/techcourse/dao/UserDaoTest.java (2)
19-41: 트랜잭션 관점에서 몇 가지 흥미로운 질문이 있습니다.Connection 리소스 관리 이슈는
tearDown()으로 잘 해결하셨네요! 👍하지만 이번 미션이 "Transaction 적용하기"라는 점을 고려하면, 테스트 코드에서도 트랜잭션 동작에 대해 생각해볼 부분이 있을 것 같아요:
현재 Connection의 auto-commit 모드는 어떻게 되어 있을까요?
- Line 26에서 얻은 Connection이 auto-commit이 켜져 있다면, 각 DAO 메서드 호출이 즉시 커밋됩니다
- 만약 꺼져 있다면, 명시적으로 commit/rollback을 해줘야 하는데 현재 코드에는 없네요
- 이게 테스트 결과에 어떤 영향을 줄까요?
모든 테스트가 하나의 Connection을 공유하는 구조인데, 테스트 격리는 괜찮을까요?
- 예를 들어 첫 번째 테스트가 Connection의 상태를 변경하면 (트랜잭션 isolation level 변경 등) 다음 테스트에 영향을 줄 수 있지 않을까요?
- 각 테스트 메서드마다 독립적인 Connection을 사용하는 것과 비교해보면 어떨까요?
테스트에서 트랜잭션 경계를 명시적으로 제어해야 할 필요는 없을까요?
- 실제 서비스 코드에서는 TransactionTemplate을 사용해 트랜잭션을 관리한다면
- 테스트에서도 이를 반영하거나, 최소한 트랜잭션 동작을 검증할 필요가 있지 않을까요?
시간 제약으로 최소한의 구현을 하셨다고 하셨는데, 이런 부분들이 바로 "고려하지 못한 흥미로운 부분"이 될 수 있을 것 같아요. 실제로 Connection의 트랜잭션 설정이 테스트 동작에 미치는 영향을 확인해보시면 좋은 학습이 될 거예요!
43-102: 트랜잭션 미션의 학습 포인트를 테스트로 검증해볼 기회는 없을까요?현재 테스트들은 DAO 메서드의 기본 동작(CRUD)을 잘 검증하고 있습니다. 하지만 "Transaction 적용하기" 미션이라면, 트랜잭션의 핵심 특성을 테스트하는 것도 의미 있을 것 같아요:
예를 들어:
- Rollback 동작 검증: 의도적으로 예외를 발생시켜서 트랜잭션이 롤백되는지 확인
- 트랜잭션 격리 확인: 커밋되지 않은 변경사항이 다른 Connection에서 보이지 않는지 검증
- All-or-Nothing: 여러 DAO 작업 중 하나가 실패하면 전부 롤백되는지 확인
현재 구조에서는 각 테스트가 독립적인 단일 DAO 작업만 수행하고 있어서, 트랜잭션의 진가가 드러나지 않는 것 같아요.
물론 이런 테스트는 통합 테스트 레벨에서 수행하는 것이 더 적절할 수 있지만, DAO 레벨에서도 트랜잭션 동작을 이해하기 위한 간단한 테스트는 추가해볼 만하지 않을까요?
참고: Line 77에서
findById(connection, 2L)로 하드코딩된 ID를 사용하고 있는데, 만약 setup에서 여러 개의 레코드를 insert하거나 다른 테스트가 먼저 실행되면 이 가정이 깨질 수 있어요. insert 후 반환된 ID를 사용하거나, account로 조회하는 방식이 더 안전할 수 있습니다.app/src/test/java/com/techcourse/service/UserServiceTest.java (2)
24-28: 인스턴스 변수 개수를 어떻게 줄일 수 있을까요?객체지향 생활 체조 규칙 7은 클래스가 최대 3개의 인스턴스 변수만 갖도록 하여 응집도를 높이고 책임을 명확히 하라고 권장합니다. 지금은 다섯 개의 필드를 공유하면서 테스트 전반의 상태 관리가 꽤 복잡해졌어요. 어떤 값들을 묶어서 하나의 협력 객체로 위임하거나, 필요한 시점에 지역 변수를 통해 생성하도록 조정을 시도해보면 어떨까요? 그렇게 하면 각각의 테스트가 필요로 하는 의존성만 선택적으로 다룰 수 있어서 이해하기 더 쉬워질 거예요.
22-80: 테스트 클래스의 책임을 더 작게 나눠볼 수 있을까요?객체지향 생활 체조 규칙 6은 한 클래스가 50줄을 넘기지 않도록 하여 단일 책임을 유지하라고 강조합니다. 현재 이 테스트 클래스는 설정, 트랜잭션 시나리오, 검증까지 모두 품고 있어서 크기가 커지고 있어요. 세부 시나리오별로 중첩 테스트 클래스를 두거나, 공통 준비 로직을 별도의 테스트 픽스처로 추출하는 방법을 고민해보시면 어떨까요? 그렇게 분리하면 각 테스트 블록이 다루는 맥락이 훨씬 선명해질 거예요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/test/java/com/techcourse/dao/UserDaoTest.java(4 hunks)app/src/test/java/com/techcourse/service/UserServiceTest.java(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.📚 학습 원칙
- 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
- 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
- 작은 단계로 문제를 분해하여 접근하도록 도와주세요
💡 피드백 방법
- 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
- 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
- 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
- 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록
⚡ 객체지향 생활 체조 원칙 검토
다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만
- 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
- 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
- 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"
규칙 2: else 예약어 금지
- else, switch/case 사용 금지
- 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
- 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"
규칙 3: 모든 원시값과 문자열 포장
- 원시 타입과 String을 객체로 포장
- 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
- 💡 예시:
int port→Port port,String name→Name name규칙 4: 한 줄에 점 하나만 (디미터 법칙)
- 메서드 체이닝 제한
- 📖 이유: 결합도 감소, 캡슐화 향상
- 💡 나쁜 예:
request.getUri().getPath().substring()- 💡 좋은 예:
request.extractPath()규칙 5: 축약 금지
- 명확한 이름 사용 (축약어 금지)
- 📖 이유: 코드 가독성, 의도 명확화
- 💡 예시:
req→request,calcAmt→calculateAmount규칙 6: 모든 엔티티를 작게 유지
- 클래스 50줄, 메서드 10줄 이하
- 📖 이유: 단일 책임, 이해와 테스트 용이성
- 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"
규칙 7: 인스턴스 변수 3개 이하
- 클래스당 최대 3개의 인스턴스 변수
- 📖 이유: 높은 응집도, 단일 책임 유지
- 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"
규칙 8: 일급 컬렉션 사용
- 컬렉션을 감싸는 클래스 사용
- 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
- 💡 예시:
List<HttpHeader>→HttpHeaders클래스규칙 9: 게터/세터/프로퍼티 금지
- Tell, Don't Ask 원칙 준수
- 📖 이유: 진정한 캡슐화, 객체 자율성
- 💡 나쁜 예:
if (user.getPassword().equals(input))- 💡 좋은 예:
if (user.authenticate(input))🎯 추가 프로그래밍 요구사항
코드 스타일
- Java Style Guide 준수
- 3항 연산자 사용 금지
테스트 및 구조
- **모든 기능 T...
Files:
app/src/test/java/com/techcourse/dao/UserDaoTest.javaapp/src/test/java/com/techcourse/service/UserServiceTest.java
🧠 Learnings (1)
📓 Common learnings
Learnt from: egaeng09
PR: woowacourse/java-jdbc#1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.
🧬 Code graph analysis (2)
app/src/test/java/com/techcourse/dao/UserDaoTest.java (1)
app/src/test/java/com/techcourse/dao/TestDataSourceConfig.java (1)
TestDataSourceConfig(6-14)
app/src/test/java/com/techcourse/service/UserServiceTest.java (4)
jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java (1)
TransactionTemplate(7-30)app/src/main/java/com/techcourse/config/DataSourceConfig.java (1)
DataSourceConfig(7-27)app/src/main/java/com/techcourse/dao/UserDao.java (1)
UserDao(9-55)app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
UserHistoryDao(9-29)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java (2)
38-51: Connection 기반 메서드의 리소스 관리 책임이 명확한가요?새로 추가된
update(Connection connection, ...)메서드는 Connection을 받아서 사용하지만 닫지 않습니다. 이는 트랜잭션 관리를 위해 의도된 설계로 보이는데요, 몇 가지 고민해볼 점이 있습니다:
- 이 메서드를 호출하는 쪽에서 Connection 생명주기 관리 책임을 명확히 인지할 수 있을까요?
- JavaDoc이나 메서드명으로 "이 Connection은 호출자가 관리해야 한다"는 것을 더 명확히 할 방법은 없을까요?
- Connection을 받지 않는 기존 메서드들과의 차이점이 API 사용자에게 명확하게 전달될까요?
💡 힌트: Connection을 파라미터로 받는 메서드와 그렇지 않은 메서드의 책임 차이를 메서드명이나 문서화로 더 명확히 구분하면 좋을 것 같습니다.
87-107: 코드 중복을 제거할 수 있을까요?Lines 87-107의
query(Connection, ...)메서드와 Lines 109-130의query(String, ...)메서드를 보면 PreparedStatement 생성, 파라미터 바인딩, ResultSet 처리 로직이 거의 동일합니다.생각해볼 점:
- Lines 109-130의 메서드가 내부적으로 Lines 87-107의 메서드를 호출하도록 리팩토링하면 어떨까요?
- Connection 획득/반납 로직만 분리하고 나머지는 공통 로직으로 추출할 수 있지 않을까요?
// 예를 들어, 이런 구조는 어떨까요? public <T> List<T> query(String sql, RowMapper<T> rowMapper, Object... params) { try (Connection connection = dataSource.getConnection()) { return query(connection, sql, rowMapper, params); } catch (SQLException e) { throw new DataAccessException(e); } }Also applies to: 109-130
app/src/main/java/com/techcourse/service/UserService.java (1)
44-47: private 헬퍼 메서드로 트랜잭션 내 조회 로직을 분리한 것이 좋습니다.
getByIdInTransaction이 있고getById도 있는 상황인데, 이 둘의 차이가 메서드명으로 명확히 드러나고 있습니다.다만 고민해볼 점:
- 두 메서드가 거의 동일한 로직을 수행하는데, 이 중복을 어떻게 보시나요?
- 퍼블릭 API와 트랜잭션 내부 API를 분리하는 것이 좋은지, 아니면 하나로 통합하되 TransactionTemplate 사용 여부를 호출부에서 결정하게 하는 것이 좋을지 생각해보셨나요?
💡 힌트: 일부 프레임워크는
@Transactional같은 선언적 방식으로 이 문제를 해결합니다. 현재 구조에서는 어떤 트레이드오프가 있을까요?app/src/test/java/com/techcourse/service/UserServiceTest.java (1)
30-41: setUp에서 Connection을 테스트 전체 기간 동안 열어두는 것이 필요한가요?Line 33에서 Connection을 얻어 필드에 저장하고, Line 40에서 한 번만 사용한 뒤 tearDown까지 열어두고 있습니다.
생각해볼 점:
- 이 Connection은 한 번만 사용되는데, 굳이 필드로 관리할 필요가 있을까요?
- Line 40의 insert도 TransactionTemplate을 통해 실행하면 어떨까요? 그러면 Connection 필드 자체가 불필요해집니다.
- 테스트에서도 프로덕션 코드와 동일한 방식(TransactionTemplate)으로 데이터를 설정하는 것이 일관성 있지 않을까요?
// 이런 방식은 어떨까요? @BeforeEach void setUp() { dataSource = DataSourceConfig.getInstance(); transactionTemplate = new TransactionTemplate(dataSource); this.jdbcTemplate = new JdbcTemplate(dataSource, new TypeConversionService()); this.userDao = new UserDao(jdbcTemplate); DatabasePopulatorUtils.execute(dataSource); final var user = new User("gugu", "password", "[email protected]"); transactionTemplate.execute(connection -> userDao.insert(connection, user)); }이렇게 하면 Connection 필드와 tearDown 메서드가 모두 불필요해집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/techcourse/dao/UserDao.java(3 hunks)app/src/main/java/com/techcourse/service/UserService.java(1 hunks)app/src/test/java/com/techcourse/service/UserServiceTest.java(2 hunks)jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java(4 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectQueryBuilder.java(2 hunks)jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectWhereStep.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectQueryBuilder.java
- app/src/main/java/com/techcourse/dao/UserDao.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 🎯 코드 품질 중심 리뷰 가이드라인이 리뷰는 코드 품질과 객체지향 원칙에 집중합니다.
미션 달성 여부가 아닌, 코드 설계와 품질 개선에 대한 피드백을 한글로 제공해주세요.📚 학습 원칙
- 직접 코드를 제공하지 마세요 (학습자가 명시적으로 요청하는 경우 제외)
- 문제 해결 과정을 안내하되, 정답을 바로 알려주지 마세요
- 작은 단계로 문제를 분해하여 접근하도록 도와주세요
💡 피드백 방법
- 유도 질문 활용: "만약 ~라면 어떻게 될까요?", "~를 고려해보셨나요?"
- 힌트 제공: 방향은 제시하되, 구체적인 구현은 학습자가 하도록
- 다양한 접근법 제시: 한 가지 해결책이 아닌 여러 가능성을 제안
- 왜?에 집중: 단순히 무엇이 잘못되었는지보다 왜 그런지 이해하도록
⚡ 객체지향 생활 체조 원칙 검토
다음은 객체지향 생활 체조(Object Calisthenics) 9가지 원칙입니다.
위반 시 학습 효과를 위해 반드시 피드백을 제공하되, 왜 이 원칙이 중요한지 설명해주세요:규칙 1: 한 메서드에 오직 한 단계의 들여쓰기만
- 들여쓰기 depth 최대 1 (중첩 제어구조 금지)
- 📖 이유: 메서드 복잡도 감소, 단일 책임 원칙 강화
- 💡 힌트: "이 부분을 별도 메서드로 추출하면 어떨까요?"
규칙 2: else 예약어 금지
- else, switch/case 사용 금지
- 📖 이유: 복잡한 분기 제거, 명확한 코드 흐름
- 💡 힌트: "early return이나 가드 클로즈 패턴을 고려해보세요"
규칙 3: 모든 원시값과 문자열 포장
- 원시 타입과 String을 객체로 포장
- 📖 이유: 도메인 개념 명확화, 비즈니스 로직 응집
- 💡 예시:
int port→Port port,String name→Name name규칙 4: 한 줄에 점 하나만 (디미터 법칙)
- 메서드 체이닝 제한
- 📖 이유: 결합도 감소, 캡슐화 향상
- 💡 나쁜 예:
request.getUri().getPath().substring()- 💡 좋은 예:
request.extractPath()규칙 5: 축약 금지
- 명확한 이름 사용 (축약어 금지)
- 📖 이유: 코드 가독성, 의도 명확화
- 💡 예시:
req→request,calcAmt→calculateAmount규칙 6: 모든 엔티티를 작게 유지
- 클래스 50줄, 메서드 10줄 이하
- 📖 이유: 단일 책임, 이해와 테스트 용이성
- 💡 힌트: "이 클래스가 너무 많은 일을 하고 있지 않나요?"
규칙 7: 인스턴스 변수 3개 이하
- 클래스당 최대 3개의 인스턴스 변수
- 📖 이유: 높은 응집도, 단일 책임 유지
- 💡 힌트: "관련 필드들을 별도 객체로 묶을 수 있을까요?"
규칙 8: 일급 컬렉션 사용
- 컬렉션을 감싸는 클래스 사용
- 📖 이유: 컬렉션 로직 캡슐화, 불변성 보장
- 💡 예시:
List<HttpHeader>→HttpHeaders클래스규칙 9: 게터/세터/프로퍼티 금지
- Tell, Don't Ask 원칙 준수
- 📖 이유: 진정한 캡슐화, 객체 자율성
- 💡 나쁜 예:
if (user.getPassword().equals(input))- 💡 좋은 예:
if (user.authenticate(input))🎯 추가 프로그래밍 요구사항
코드 스타일
- Java Style Guide 준수
- 3항 연산자 사용 금지
테스트 및 구조
- **모든 기능 T...
Files:
app/src/test/java/com/techcourse/service/UserServiceTest.javaapp/src/main/java/com/techcourse/service/UserService.javajdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.javajdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectWhereStep.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: egaeng09
Repo: woowacourse/java-jdbc PR: 1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.
📚 Learning: 2025-10-21T09:53:41.119Z
Learnt from: egaeng09
Repo: woowacourse/java-jdbc PR: 1181
File: app/src/main/java/com/techcourse/dao/UserHistoryDao.java:15-26
Timestamp: 2025-10-21T09:53:41.119Z
Learning: egaeng09의 java-jdbc 프로젝트에서는 도메인 모델의 순수성을 우선시하며, 도메인 객체가 JDBC와 같은 기술적 구현체에 의존하지 않도록 합니다. DAO 계층에서 도메인 객체의 getter를 사용하는 것은 계층 분리를 유지하기 위해 허용됩니다.
Applied to files:
app/src/main/java/com/techcourse/service/UserService.java
🧬 Code graph analysis (2)
app/src/test/java/com/techcourse/service/UserServiceTest.java (4)
jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java (1)
TransactionTemplate(7-30)app/src/main/java/com/techcourse/config/DataSourceConfig.java (1)
DataSourceConfig(7-27)app/src/main/java/com/techcourse/dao/UserDao.java (1)
UserDao(9-63)app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
UserHistoryDao(9-29)
app/src/main/java/com/techcourse/service/UserService.java (3)
jdbc/src/main/java/com/interface21/transaction/support/TransactionTemplate.java (1)
TransactionTemplate(7-30)app/src/main/java/com/techcourse/dao/UserDao.java (1)
UserDao(9-63)app/src/main/java/com/techcourse/dao/UserHistoryDao.java (1)
UserHistoryDao(9-29)
🔇 Additional comments (6)
jdbc/src/main/java/com/interface21/jdbc/core/querybuilder/select/SelectWhereStep.java (1)
11-19: 인터페이스 확장이 일관성 있게 잘 이루어졌습니다.Connection 파라미터를 받는 오버로드와 받지 않는 오버로드를 명확히 구분하고, 주석으로 각각의 용도를 설명한 점이 좋습니다. JdbcTemplate의 패턴과도 일관성이 있네요.
app/src/main/java/com/techcourse/service/UserService.java (3)
24-27: getById는 트랜잭션 없이 조회하는 것이 의도된 설계인가요?Line 24-27의
getById메서드는 TransactionTemplate을 사용하지 않는 반면, Line 44-47의getByIdInTransaction은 트랜잭션 내에서 조회합니다.생각해볼 점:
- 단순 조회 작업에 트랜잭션이 필요한 경우는 언제일까요?
changePassword처럼 여러 작업이 하나의 트랜잭션으로 묶여야 할 때 일관된 데이터를 읽어야 하지 않을까요?- 현재 구조에서
getById는 auto-commit 모드로 동작하는데, 이것이 의도한 격리 수준(isolation level)을 제공하나요?💡 힌트: 읽기 작업도 다른 쓰기 작업과 격리되어야 하는 경우가 있습니다. 트랜잭션 경계를 어디까지 설정할지는 비즈니스 요구사항에 따라 달라질 수 있어요.
29-33: insert 메서드에 트랜잭션이 적용되어 좋습니다!이전 리뷰에서 songsunkook님이 질문하신 부분이 해결되었네요. 단일 insert 작업이지만 TransactionTemplate을 통해 명시적으로 트랜잭션 경계를 관리하는 것이 일관성 있는 설계입니다.
다만 한 가지 더 생각해볼 점:
- 현재
insert는 단일 삽입 작업인데, 향후 이 메서드에서 검증 로직이나 부가 작업이 추가될 가능성이 있나요? 그렇다면 현재의 트랜잭션 구조가 확장에 유리할 것 같습니다.
35-42: 트랜잭션 로직이 훨씬 깔끔해졌습니다!이전의 중첩된 try-catch 구조에서 TransactionTemplate을 사용한 구조로 개선되어 객체지향 생활 체조 원칙(규칙 1: 한 단계 들여쓰기, 규칙 6: 메서드 10줄 이하)을 잘 지키고 있습니다.
특히 좋은 점:
- 트랜잭션 관리 책임이 TransactionTemplate으로 명확히 분리되었습니다
- 비즈니스 로직(사용자 조회 → 비밀번호 변경 → 업데이트 → 히스토리 기록)이 선형적으로 읽힙니다
- 들여쓰기 깊이가 1단계로 유지되어 가독성이 높습니다
app/src/test/java/com/techcourse/service/UserServiceTest.java (2)
43-48: 리소스 관리가 개선되었습니다!이전 리뷰에서 지적되었던 Connection 리소스 누수 문제가 tearDown 메서드 추가로 해결되었네요. null 체크도 포함되어 있어 안전합니다.
다만, 위 코멘트에서 제안한 것처럼 setUp에서 TransactionTemplate을 사용한다면 이 tearDown 자체가 불필요해질 수 있습니다.
50-62: 테스트가 명확하게 작성되었습니다.TransactionTemplate을 통해
changePassword가 실행되고, 결과를getById로 검증하는 흐름이 명확합니다.한 가지 질문:
- Line 59에서
getById를 사용하는데, 이는 별도의 Connection을 사용하는 auto-commit 조회입니다. 만약 트랜잭션이 아직 commit되지 않았다면 어떻게 될까요? (현재는 TransactionTemplate이 commit한 후이므로 문제없지만, 트랜잭션 격리 수준에 대해 생각해볼 기회입니다)
2 participants
안녕하세요 모코!
시간 이슈로 미션지에서 제공하는 최소한의 코드로 작성했습니다.
제가 고려해보지 못한 흥미로운 부분이 있다면 언제든 코멘트 주세요!