4단계 - 수강신청(요구사항 변경) #786
4단계 - 수강신청(요구사항 변경) #786
Conversation
- Enrollment 추가
| - [x] Enrollment.hasEnrolledUser 제거 | ||
| # 4단계 |
| this.id = id; | ||
| this.info = info; | ||
| this.enrollment = enrollment; | ||
| this.enrollments = enrollments; | ||
| } | ||
|
|
||
| public void enroll(NsUser user, Payment payment) { |
There was a problem hiding this comment.
| public void enroll(NsUser user, Payment payment) { | |
| public Enrollments enrollments(NsUser user, Payment payment) { |
Enrollments을 Session의 인스턴스 변수로 구현하지 말고 Session의 정보들을 활용해 Enrollments을 생성하도록 구현하는 것은 어떨까?
| SessionStatus getStatus(); | ||
| public class Enrollment { | ||
| private final NsUser user; | ||
| private EnrollmentStatus enrollmentStatus; |
| if (enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL) { | ||
| enrollmentStatus = EnrollmentStatus.ENROLLED; | ||
| } else { | ||
| throw new IllegalStateException("승인 대기 상태의 수강신청만 승인할 수 있습니다."); | ||
| } |
There was a problem hiding this comment.
| if (enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL) { | |
| enrollmentStatus = EnrollmentStatus.ENROLLED; | |
| } else { | |
| throw new IllegalStateException("승인 대기 상태의 수강신청만 승인할 수 있습니다."); | |
| } | |
| enrollmentStatus = enrollmentStatus.approve(); |
이 로직 구현을 EnrollmentStatus에 구현하는 것은 어떨까?
| if (enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL || | ||
| enrollmentStatus == EnrollmentStatus.ENROLLED) { | ||
| enrollmentStatus = EnrollmentStatus.CANCELLED; | ||
| } else { | ||
| throw new IllegalStateException("승인 대기 또는 수강신청 상태만 취소할 수 있습니다."); | ||
| } |
| public boolean isPendingApproval() { | ||
| return enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL; | ||
| } | ||
|
|
||
| public boolean isEnrolled() { | ||
| return enrollmentStatus == EnrollmentStatus.ENROLLED; | ||
| } | ||
|
|
||
| public boolean isCancelled() { | ||
| return enrollmentStatus == EnrollmentStatus.CANCELLED; | ||
| } |
There was a problem hiding this comment.
이 3개의 메서드를 모두 EnrollmentStatus에 구현하는 것은 어떨까?
enum도 값을 꺼내기 보다는 enum에 메시지를 보내 구현한다.
|
|
||
| @Getter | ||
| public class EnrollmentManager { |
There was a problem hiding this comment.
FreeEnrollments와 PaidEnrollments의 중복을 EnrollmentManager을 통해 해결하는 것으로 보여진다.
FreeEnrollments와 PaidEnrollments의 중복을 Enrollments 사이에 AbstractEnrollments를 두어 중복을 제거하는 것은 어떨까?
| new SessionId(1L, 1L), | ||
| sessionInfo, | ||
| new FreeEnrollments() | ||
| ); |
There was a problem hiding this comment.
Session 객체와 같이 인스턴스 변수가 많은 객체를 테스트하려면 객체를 생성하는데 어려움이 있다.
중복 코드 또한 많이 발생해 Session을 생성할 때 생성자의 인자가 변경되는 경우 변경할 부분이 많아진다.
https://www.arhohuttunen.com/test-data-builders/ 문서 참고해 Session과 같이 복잡한 객체의 테스트 데이터를 생성할 때 어떤 방법을 사용할 것인지 선택해 보면 좋겠다.
이번 기회에 내가 선호하는 방법을 적용해 보고 앞으로도 쭈욱 활용하는 방식이면 좋겠다.
안녕하세요, 과정은 모두 끝났지만 PR이 가능하다고 하여 올려봅니다. 이번 단계에서는 유/무료 강의를 구분하는 설계에 대하여 많이 고민했습니다. 수강 신청을 관리하는 Enrollments 객체 단계에서 유/무료 강의를 구분하는 것이 옳다고 생각하여 Enrollments를 인터페이스로 구현했습니다.
그런데 Enrollment는 강의 객체인 Session의 필드라는 걸 생각하면 Session을 인터페이스로 구현하는 게 맞나? 라는 생각도 드네요.
그리고 Enrollments 인터페이스를 구현한 구현체는 모두 공통적인 필드를 갖는데 인터페이스는 필드를 가질 수 없습니다. 그렇다고 필드를 강제하기 위해 추상 클래스를 쓰는 것은 지양하고 싶었구요. 그래서 생각한 방법이 공통된 필드를 갖는 EnrollmentManager를 컴포지션 형태로 받아서 사용하는 방법입니다. 그런데, 지금 생각해보니 결국 EnrollmentManager도 반드시 포함하도록 제한하는 수단은 없으니 이 문제를 완벽히 해결한 것은 아니네요. 의견 있다면 부탁 드립니다.
마지막까지 리뷰 제공해주셔서 감사드립니다.