-
Notifications
You must be signed in to change notification settings - Fork 308
2단계 - 수강신청(도메인 모델) #818
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
base: username0w
Are you sure you want to change the base?
2단계 - 수강신청(도메인 모델) #818
Conversation
javajigi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금까지 학습한 내용을 바탕으로 2단계 미션 진행 💯
현 상태로 바로 merge해도 되는데요.
db 매핑하기 전에 한번 더 고민해 봤으면 하는 내용이 있어 피드백 남겨 봅니다.
추가로 sessions/domin 패키지에 도메인 객체 수가 많아지는데요.
패키지 분리 리팩터링도 이번 기회에 도전해 보면 어떨까요?
|
|
||
| private final Integer maxCapacity; | ||
| private final boolean unlimited; | ||
| private final int enrollCount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
db가 아닌 도메인 객체인 만큼 수강생 목록을 가지는 것은 어떨까?
db를 고려해 구현한 것은 아닐까?
|
|
||
| import java.time.LocalDate; | ||
|
|
||
| public class Session { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Session의 필드 수가 많다.
Session의 기본 정보를 가지는 필드(현재 SessionInfo)와 수강신청에 관여하는 필드 값을 하나의 객체로 묶은 후 수강 신청 로직을 이 객체가 담당하도록 역할을 부여하는 것은 어떨까?
|
|
||
| import java.time.LocalDate; | ||
|
|
||
| class SessionTestBuilder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
안녕하세요.
2단계 - 수강신청(도메인 모델) PR 입니다.