Step3 리뷰요청 드립니다. #824
Open
Step3 리뷰요청 드립니다. #824
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Jdbc 연결을 위한 get 메소드 추가 - CoverImage fileName 필드 추가 - EnrollmentPolicy 필드 저장을 위한 데이터 추가(type, price) - Test 코드 구현
- 인터페이스 생성 - 구현 로직 생성 - 테스트코드 구현
- domain/repository/service로 구분
- SessionService 구현(repository 묶음)
javajigi
requested changes
Dec 26, 2025
Contributor
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
db 매핑 작업 넘 잘 했네요. 👍
기존의 도메인 객체를 최대한 유지하면서 적용한 점이 인상적이네요.
굳이 image와 같은 부분에 대한 작업은 하지 않아도 될 수 있는데요.
이 부분이 4단계 미션에 영향을 미치는 부분이 있어서요.
session외에 다른 부분까지 db 매핑한 후에 미션 마무리하고 4단계 진행하면 좋겠습니다.
추가로 domain 패키지에 도메인 클래스가 증가하고 있네요.
domain 패키지를 분리하는 도전도 추가로 진행해 보면 좋겠습니다.
| FREE, | ||
| PAID; | ||
|
|
||
| public static EnrollmentPolicy create(String name, Long price) { |
Contributor
There was a problem hiding this comment.
EnrollmentPolicy 생성을 PolicyType이 담당하기 보다 별도의 EnrollmentPolicyFactory와 같은 객체를 추가하는 것은 어떨까?
| this.enrollmentRepository = enrollmentRepository; | ||
| } | ||
|
|
||
| public void enroll(Long sessionId, Long userId, Payment payment) { |
| public Enrollment enroll(Long userId, Payment payment) { | ||
| sessionState.validateEnroll(); | ||
| enrollmentPolicy.validateEnrollment(payment); | ||
| return enrollments.add(this.id, userId); |
Contributor
There was a problem hiding this comment.
메서드 이름이 add이면 반환 타입을 void로 예상할 가능성도 있어보임
AI와의 협업을 통해 더 적합한 이름이 있는지 피드백을 받아 적용해 본다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요! 메리크리스마스입니다.
3단계 미션 진행해보았습니다.
아직 더 진행해야하는 부분이 있는거 같기도한데, 우선 Session에 집중해서 구현해보았습니다.
고민됬던 부분은 이렇습니다.
변경을 최소화하려고 했는데, 일부 추가된 부분이 있습니다.
특히 jdbc에 데이터를 넣기 위해서 get 메소드가 많이 추가되었는데요.. 뭔가 찝찝했지만, 필요한거 같아서 추가했습니다.
그리고 SessionRepository에 Enrollment도 넣으려다가 따로 구현하고, 서비스를 만들어서 각각의 Repository를 묶었습니다.
요 방향이 맞는건지...확신은 없지만 서비스로 구현해두니 어느정도 계층이 구별되는거 같아보여요.
( 이렇게 domain/repository/service를 구별해서 실무에서도 개발하면 더 재밌을거 같습니다. )
좀 어설픈거 같아서.. 확인 부탁드리고, 피드백 주시면 더 깊게 고민해보겠습니다!