🚀 2단계 - 수강신청(도메인 모델) #732
🚀 2단계 - 수강신청(도메인 모델) #732
Conversation
| public class Enrollment { | ||
| private final NsUser student; | ||
| private final Session session; | ||
| private final LocalDateTime enrolledAt; | ||
| private final Payment payment; |
There was a problem hiding this comment.
이 객체의 역할은 vaildator 에 가까운 것 같은데요.
student 변수는 활용되는 곳이 없어서 제거하면 어떨까 싶어요.
아직 미구현 단계라도 예상되는 코드는 제거하고 적은 역할만 부여해보셔도 좋을 것 같아요.
그리고 수강 신청이라는 기능은 Session의 기능이니 sessions 패키지 하위로 이동하면 어떨까요?
sessions와 동일한 레벨에 위치하신 이유가 궁금합니다 🤔
There was a problem hiding this comment.
@testrace
좋은 피드백 감사합니다!
말씀 주신 validator 역할이라는 관점도 공감이 가지만, 저는 Enrollment를 학생과 수업 간의 다대다 관계를 명시적으로 표현하는 도메인 객체로 보고 있습니다.
수강신청은 단순히 유효성 검사를 넘어서, "누가 어떤 수업에 얼마를 지불하고 신청했는가"라는 도메인 이벤트를 포착하고 있다고 생각했습니다.
패키지 구조에 대해서도, Enrollment가 단순히 Session의 하위 기능이라기보다는, 학생과 수업 사이의 독립된 관계이자 행위라고 보아 enrollment 패키지로 따로 분리했습니다.
혹시 수강신청에 대해서 제가 잘못이해하고 있다면 알려주시면 감사하겠습니다.
There was a problem hiding this comment.
수강신청 자체에 대해선 잘못이해하신 것은 없으신 것 같아요 😃
수강신청 미션은 이벤트 기반 방식은 고려하지 않았기 때문에 (사실 모든 미션이)
도메인 이벤트 객체라고 인지하지 않아서 Enrollment 의 역할을 단순히 validator 라고 생각했습니다.
이벤트를 위한 객체라면 클래스명을 EnrolledEvent 로 변경하면 어떨까 싶어요.
패키지 구조에서 말씀하신 �학생과 수업 사이의 독립된 관계이자 행위를 이해하지 못했는데요.
그럼 session 하위 기능으로는 둘 수 없다는 의견이신지, 둘 다 가능하지만 독립적인 객체로 설계 하신건지 조금 더 자세하게 설명 부탁드려요.
| public Long amount() { | ||
| return this.amount; | ||
| } |
There was a problem hiding this comment.
금액을 비교하기 위한 메서드라면 메시지를 보내 검증하면 어떨까요?
There was a problem hiding this comment.
@testrace
좋은 피드백 감사합니다! 코드를 짜면서 고민이 있었는데요.
'수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능하다'라는 요구사항을 처리하면서, 해당 비교 책임을 어느 객체에 둘지에 대해 고민이 있었습니다.
Payment에 수업 가격을 전달하여 내부에서 비교하는 방식도 고려했지만,
이 경우 결국 Payment가 Session의 가격 정보를 알아야 하며(Session은 결국 getter를 통해 값을 외부로 노출),
한쪽 도메인 객체가 다른 도메인 객체의 맥락에 의존하게 되는 구조가 되어버린다고 생각했습니다.
반면 Enrollment는 수강신청이라는 행위를 중심으로 student, session, payment를 모두 알고 있는 도메인 객체이기 때문에, 이곳에서 두 객체의 상태를 조회해 비교하는 것이 양쪽 도메인의 책임을 침범하지 않으면서도 자연스럽다고 판단했습니다.
혹시 이렇게 두 객체의 값을 비교하는 상황에서는 어떻게 하는게 좋을까요??
There was a problem hiding this comment.
Session(또는 하위 도메인) 내부에서 payment 객체를 주입받아서 메시지를 보내도 괜찮지 않을까요?
payment.hasPaidCorrectly(price);
Enrollment가 행위 중심이라고 하셨는데 어떤 행위를 갖고 있나요?
validate 말고는 행위라고 판단할 수 있는 메서드는 없는 것 같아서요.
현재 구조에선 수강료와 결제금액을 비교하기 위해 도메인 객체가 다른 도메인 객체에게 의존하는 구조가 나쁘다고 생각하진 않습니다.
의존 방향만 잘 설정하면 특별히 문제가 되진 않을 것 같아요.
그럼에도 의존성을 완전히 제거해야 한다면 '도메인 서비스'를 두면 어떨까요?
이미 Enrollment 가 그 역할을 하고 있는데 이벤트를 위한 객체가 아닌 금액 비교를 위한 서비스를 추가해도 좋을 것 같아요.
| public class EnrollmentCapacity { | ||
| private int maxEnrollment; | ||
| private int currentEnrollment; |
There was a problem hiding this comment.
캡슐화 👍
수강생 목록을 가지면 어떨까요?
중복신청도 처리할 수 있을 것 같아요.
| package nextstep.sessions.domain; | ||
|
|
||
| public class Image { | ||
| private static final Long MAX_SIZE_BYTES = 1_048_576L; |
There was a problem hiding this comment.
1024 * 1024로 표현해 보셔도 좋을 것 같아요.
| private static final Float MIN_SIZE_WIDTH = 300F; | ||
| private static final Float MIN_SIZE_HEIGHT = 200F; |
| public Session(Long price) { | ||
| this.price = price; | ||
| } | ||
|
|
||
| public Session(SessionStatus status, Long price) { | ||
| this.status = status; | ||
| this.price = price; | ||
| } |
| private Date startDate; | ||
| private Date endDate; |
There was a problem hiding this comment.
final 키워드를 추가해 불변성을 보장하면 어떨까요?
더불어 java8의 LocalDate 를 활용해 보셔도 좋을 것 같아요.
| @Test | ||
| @DisplayName("결제 금액이 강의 수강료와 일치하면 수강 신청에 성공한다.") | ||
| void 결제_금액_강의_금액_일치() { | ||
| new Enrollment(new NsUser(), new Session(10_000L), new Payment("user_id", 0L, 0L, 10_000L)); |
There was a problem hiding this comment.
ImageTypeTest를 제외하곤 성공 테스트의 경우 assertion이 없네요.
최소한의 구현이라 없는 것인지, 객체가 생성된 것만으로 테스트의 의도를 검증된다고 생각하시는지 궁금합니다 🤔
There was a problem hiding this comment.
@testrace
좋은 피드백 감사합니다!
객체가 생성된 것만으로 테스트의 의도가 검증된다고 생각했었는데 그게 아니었나보군요.
혹시 equals를 활용한 객체의 비교나 assertDoesNotThrow() 같은 메서드를 활용해야할까요?
There was a problem hiding this comment.
객체의 역할을 서로 다르게 해석했기에 생성만으로 검증이 충분하다고 판단하신 것 같아요 😃
생성외엔 기능이 없다면 말씀하신대로 equals, assertDoesNotThrow 검증을 하면 좋을 것 같아요.
다만, 학습 목표가 객체 설계와 구현이니 적절한 책임을 가진 도메인 객체를 도출해서 '기능'을 검증해 보시면 좋을 것 같습니다.
예를 들어 '신청에 성공하면 수강생 정보를 반환한다'가 될 수 있겠네요.
Enrollment enrollment = new Enrollment();
Student student = enrollment.enroll(user, payment);
assertThat(student).equals(new Student(...));이벤트를 고려하셨다니 도메인간 결합을 최대한 끊으려 하신 것 같은데요.
다양한 시도는 좋지만 학습 목표와는 조금 거리가 있는 느낌을 받았습니다.
수강신청 미션도 자동차, 로또, 사다리와 다르지 않게 단순한 단일 애플리케이션입니다.
지난 미션들과 같은 규칙과 원칙들을 적용해서 구현해 보시면 어떨까 싶어요.
이벤트 기반을 적용해 보고 싶으시다면 4단계에서 해보시는 것을 추천드립니다.
(혹시 제가 잘못 이해했다면 코멘트 꼭 남겨주세요 😃 )
| @Test | ||
| @DisplayName("정원이 가득 찬 경우 인원 증가 시 예외를 던진다.") | ||
| void 등록_인원_초과() { | ||
| EnrollmentCapacity enrollmentCapacity = new EnrollmentCapacity(1, 1); | ||
| assertThatThrownBy(enrollmentCapacity::increaseEnrollment) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessage("정원이 초과되었습니다."); | ||
| } |
There was a problem hiding this comment.
요구사항에 따라 무료 강의는 정원 제한이 없기에 EnrollmentCapacity#increaseEnrollment 는 예외를 던지지 않아야 할 것 같아요.
무료강의에 대한 테스트도 추가하면 어떨까요?
더불어 예외 검증 시 assertThatThrownBy, assertThatIllegalArgumentException 가 혼용되고 있는데
일관성을 위해 하나의 assertThat~ 메서드로 통일하면 어떨까요?
| private void validatePaidCorrectly(Session session, Payment payment) { | ||
| if (!hasPaidCorrectly(session, payment)) { | ||
| throw new IllegalArgumentException("결제 금액이 수강료와 일치하지 않습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 2단계를 아직 완전히 완료하지는 못했습니다.
진행하면서 요구사항 중 어디까지 구현해야 하는지 조금 헷갈리는 부분이 있어,
우선 도메인 단위로 제가 생각한 요구사항을 만족하는 최소한의 기능만 구현해둔 상태입니다.
혹시 이 상태에서 서비스 레이어까지 구현을 확장해도 될지 궁금합니다.
또한, 현재는 결제와 수강신청을 별개의 로직으로 분리해두었는데요,
수강신청을 결제의 연장선으로 보고 결제 도메인 안에서 처리하는 게 맞을지 판단이 잘 서지 않아 고민 중입니다.
프레임워크가 적용되어도 크게 다르지 않으니 지난 미션처럼 TDD 사이클로 기능 구현하시고,
진입점을 main 메서드가 아닌 서비스 레이어로 구현해 주시면 됩니다.
수강 신청이 결제 이후 진행되는 것은 맞지만 결제 도메인이 수강신청의 책임을 가질 필요는 없다고 생각합니다.
결제 도메인이 처리를 한다면 추후 결제가 필요한 다른 기능이 생길 때 결제 도메인은 너무 많은 것을 알고 있어야 합니다.
그러니 결제 도메인에서 수강신청 기능을 처리하기 보다 지금처럼 분리해 주시는게 좋습니다 😃
안녕하세요, 2단계도 잘 부탁드립니다!
현재 2단계를 아직 완전히 완료하지는 못했습니다.
진행하면서 요구사항 중 어디까지 구현해야 하는지 조금 헷갈리는 부분이 있어,
우선 도메인 단위로 제가 생각한 요구사항을 만족하는 최소한의 기능만 구현해둔 상태입니다.
혹시 이 상태에서 서비스 레이어까지 구현을 확장해도 될지 궁금합니다.
또한, 현재는 결제와 수강신청을 별개의 로직으로 분리해두었는데요,
수강신청을 결제의 연장선으로 보고 결제 도메인 안에서 처리하는 게 맞을지 판단이 잘 서지 않아 고민 중입니다.
이 부분에 대한 의견도 주시면 감사하겠습니다!