-
Notifications
You must be signed in to change notification settings - Fork 301
🚀 2단계 - 수강신청(도메인 모델) #732
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: jieung2
Are you sure you want to change the base?
🚀 2단계 - 수강신청(도메인 모델) #732
Changes from all commits
330def7
45603af
7bc74ca
90f69fa
1434fba
2104de2
8c94a3d
14635ef
163d903
622012c
6d53e29
aaeb502
8a8a195
c23a134
e3f5f4d
ac80d4f
8059451
ea80407
1e6b44f
1e733eb
dd15699
3a38507
a14350f
5c177c3
ec7e477
aab7ce1
02cb05a
9265ddb
2ecb6bd
f547d74
835cf53
31f9f94
7ecb687
db7209e
cf74b39
fa79243
6fe969c
fb64a35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
## 기능 요구사항 | ||
- 과정(Course)은 기수 단위로 운영하며, 여러 개의 강의(Session)를 가질 수 있다. | ||
- 강의는 시작일과 종료일을 가진다. | ||
- 강의는 강의 커버 이미지 정보를 가진다. | ||
- 이미지 크기는 1MB 이하여야 한다. | ||
- 이미지 타입은 gif, jpg(jpeg 포함),, png, svg만 허용한다. | ||
- 이미지의 width는 300픽셀, height는 200픽셀 이상이어야 하며, width와 height의 비율은 3:2여야 한다. | ||
- 강의는 무료 강의와 유료 강의로 나뉜다. | ||
- 무료 강의는 최대 수강 인원 제한이 없다. | ||
- 유료 강의는 강의 최대 수강 인원을 초과할 수 없다. | ||
- 유료 강의는 수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능하다. | ||
- 강의 상태는 준비중, 모집중, 종료 3가지 상태를 가진다. | ||
- 강의 수강신청은 강의 상태가 모집중일 때만 가능하다. | ||
- 유료 강의의 경우 결제는 이미 완료한 것으로 가정하고 이후 과정을 구현한다. | ||
- 결제를 완료한 결제 정보는 payments 모듈을 통해 관리되며, 결제 정보는 Payment 객체에 담겨 반한된다. | ||
|
||
## 기능 목록 | ||
- [x] 강의의 종료일보다 시작일이 더 이후인지 확인한다. | ||
- [x] 강의 커버 이미지의 크기가 1MB 이하인지 확인한다. | ||
- [x] 이미지 타입이 gif, jpg, png, svg인지 확인한다. | ||
- [x] 이미지의 width와 height가 각각 300, 200 픽셀 이상인지 확인한다. | ||
- [x] 이미지의 width와 height 비율이 3:2인지 확인한다. | ||
- [x] 유료 강의의 강의 최대 수강 인원을 초과하는지 확인한다. | ||
- [x] 현재 수강 인원을 증가시킨다. | ||
- [x] 유료 강의의 수강료와 수강생이 결제한 금액이 일치하는지 확인한다. | ||
- [x] 수강 신청을 한다. | ||
- [x] 강의 상태가 모집중인지 확인한다. | ||
- [x] 결제 금액을 반환한다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package nextstep.enrollment.domain; | ||
|
||
import nextstep.payments.domain.Payment; | ||
import nextstep.sessions.domain.Session; | ||
import nextstep.users.domain.NsUser; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.Objects; | ||
|
||
public class Enrollment { | ||
private final NsUser student; | ||
private final Session session; | ||
private final LocalDateTime enrolledAt; | ||
private final Payment payment; | ||
|
||
public Enrollment(NsUser student, Session session, Payment payment) { | ||
validate(session, payment); | ||
this.student = student; | ||
this.session = session; | ||
this.enrolledAt = LocalDateTime.now(); | ||
this.payment = payment; | ||
} | ||
|
||
private void validate(Session session, Payment payment) { | ||
validateSessionStatus(session); | ||
validatePaidCorrectly(session, payment); | ||
} | ||
|
||
private void validateSessionStatus(Session session) { | ||
if (!session.isRecruiting()) { | ||
throw new IllegalArgumentException("강의 상태가 모집중일 때만 수강 신청이 가능합니다."); | ||
} | ||
} | ||
|
||
private void validatePaidCorrectly(Session session, Payment payment) { | ||
if (!hasPaidCorrectly(session, payment)) { | ||
throw new IllegalArgumentException("결제 금액이 수강료와 일치하지 않습니다."); | ||
} | ||
} | ||
Comment on lines
+35
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
프레임워크가 적용되어도 크게 다르지 않으니 지난 미션처럼 TDD 사이클로 기능 구현하시고, 수강 신청이 결제 이후 진행되는 것은 맞지만 결제 도메인이 수강신청의 책임을 가질 필요는 없다고 생각합니다. |
||
|
||
private boolean hasPaidCorrectly(Session session, Payment payment) { | ||
return Objects.equals(payment.amount(), session.price()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,4 +26,8 @@ public Payment(String id, Long sessionId, Long nsUserId, Long amount) { | |
this.amount = amount; | ||
this.createdAt = LocalDateTime.now(); | ||
} | ||
|
||
public Long amount() { | ||
return this.amount; | ||
} | ||
Comment on lines
+30
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 금액을 비교하기 위한 메서드라면 메시지를 보내 검증하면 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @testrace '수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능하다'라는 요구사항을 처리하면서, 해당 비교 책임을 어느 객체에 둘지에 대해 고민이 있었습니다. Payment에 수업 가격을 전달하여 내부에서 비교하는 방식도 고려했지만, 반면 Enrollment는 수강신청이라는 행위를 중심으로 student, session, payment를 모두 알고 있는 도메인 객체이기 때문에, 이곳에서 두 객체의 상태를 조회해 비교하는 것이 양쪽 도메인의 책임을 침범하지 않으면서도 자연스럽다고 판단했습니다. 혹시 이렇게 두 객체의 값을 비교하는 상황에서는 어떻게 하는게 좋을까요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Session(또는 하위 도메인) 내부에서 payment 객체를 주입받아서 메시지를 보내도 괜찮지 않을까요? Enrollment가 행위 중심이라고 하셨는데 어떤 행위를 갖고 있나요? 현재 구조에선 수강료와 결제금액을 비교하기 위해 도메인 객체가 다른 도메인 객체에게 의존하는 구조가 나쁘다고 생각하진 않습니다. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package nextstep.sessions.domain; | ||
|
||
public class EnrollmentCapacity { | ||
private int maxEnrollment; | ||
private int currentEnrollment; | ||
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 캡슐화 👍 |
||
|
||
public EnrollmentCapacity(int maxEnrollment) { | ||
this(maxEnrollment, 0); | ||
} | ||
|
||
public EnrollmentCapacity(int maxEnrollment, int currentEnrollment) { | ||
this.maxEnrollment = maxEnrollment; | ||
this.currentEnrollment = currentEnrollment; | ||
} | ||
|
||
public boolean isFull() { | ||
return currentEnrollment >= maxEnrollment; | ||
} | ||
|
||
public void increaseEnrollment() { | ||
validate(); | ||
currentEnrollment++; | ||
} | ||
|
||
private void validate() { | ||
if (isFull()) { | ||
throw new IllegalStateException("정원이 초과되었습니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
package nextstep.sessions.domain; | ||
|
||
public class Image { | ||
private static final Long MAX_SIZE_BYTES = 1024 * 1024L; | ||
private static final Float MIN_SIZE_WIDTH = 300F; | ||
private static final Float MIN_SIZE_HEIGHT = 200F; | ||
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. int 타입이어도 괜찮지 않을까요? |
||
private static final Float ASPECT_RATIO_TOLERANCE = 0.01f; | ||
|
||
private Long id; | ||
private Long sizeInBytes; | ||
private String title; | ||
private ImageType type; | ||
private Float width; | ||
private Float height; | ||
|
||
public Image(Long sizeInBytes, String title, ImageType type, Float width, Float height) { | ||
this(sizeInBytes, 0L, title, type, width, height); | ||
} | ||
|
||
public Image(Long sizeInBytes, Long id, String title, ImageType type, Float width, Float height) { | ||
validate(sizeInBytes, title, type, width, height); | ||
this.sizeInBytes = sizeInBytes; | ||
this.id = id; | ||
this.title = title; | ||
this.type = type; | ||
this.width = width; | ||
this.height = height; | ||
} | ||
|
||
private void validate(Long sizeInBytes, String title, ImageType type, Float width, Float height) { | ||
validateSize(sizeInBytes); | ||
validateWidth(width); | ||
validateHeight(height); | ||
validateAspectRatio(width, height); | ||
} | ||
|
||
private void validateSize(Long sizeInBytes) { | ||
if (sizeInBytes > MAX_SIZE_BYTES) { | ||
throw new IllegalArgumentException("이미지 크기는 최대 1MB 이하여야 합니다."); | ||
} | ||
} | ||
|
||
private void validateWidth(Float width) { | ||
if (width < MIN_SIZE_WIDTH) { | ||
throw new IllegalArgumentException("이미지 가로 길이는 최소 300픽셀 이상이어야 합니다."); | ||
} | ||
} | ||
|
||
private void validateHeight(Float height) { | ||
if (height < MIN_SIZE_HEIGHT) { | ||
throw new IllegalArgumentException("이미지 세로 길이는 최소 200픽셀 이상이어야 합니다."); | ||
} | ||
} | ||
|
||
private void validateAspectRatio(float width, float height) { | ||
float ratio = width / height; | ||
if (Math.abs(ratio - (MIN_SIZE_WIDTH / MIN_SIZE_HEIGHT)) > ASPECT_RATIO_TOLERANCE) { | ||
throw new IllegalArgumentException("이미지 비율은 3:2이어야 합니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package nextstep.sessions.domain; | ||
|
||
public enum ImageType { | ||
GIF, JPG, JPEG, PNG, SVG; | ||
|
||
public static ImageType from(String type) { | ||
try { | ||
return ImageType.valueOf(type.toUpperCase()); | ||
} catch (IllegalArgumentException e) { | ||
throw new IllegalArgumentException("지원하지 않는 이미지 타입입니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package nextstep.sessions.domain; | ||
|
||
public class Session { | ||
private Long id; | ||
private String title; | ||
private SessionType type; | ||
private SessionStatus status; | ||
private SessionPeriod period; | ||
private Image coverImage; | ||
private EnrollmentCapacity enrollmentCapacity; | ||
private Long price; | ||
Comment on lines
+3
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
public Session(Long price) { | ||
this.price = price; | ||
} | ||
|
||
public Session(SessionStatus status, Long price) { | ||
this.status = status; | ||
this.price = price; | ||
} | ||
Comment on lines
+13
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 주 생성자를 호출해 보시면 좋을 것 같아요. |
||
|
||
public Session(Long id, String title, SessionType type, SessionStatus status, SessionPeriod period, Image coverImage, EnrollmentCapacity enrollmentCapacity, Long price) { | ||
this.id = id; | ||
this.title = title; | ||
this.type = type; | ||
this.status = status; | ||
this.period = period; | ||
this.coverImage = coverImage; | ||
this.enrollmentCapacity = enrollmentCapacity; | ||
this.price = price; | ||
} | ||
|
||
public Long price() { | ||
return this.price; | ||
} | ||
|
||
public boolean isRecruiting() { | ||
return status.equals(SessionStatus.RECRUITING); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package nextstep.sessions.domain; | ||
|
||
import java.time.LocalDate; | ||
|
||
public class SessionPeriod { | ||
private final LocalDate startDate; | ||
private final LocalDate endDate; | ||
|
||
public SessionPeriod(LocalDate startDate, LocalDate endDate) { | ||
validate(startDate, endDate); | ||
this.startDate = startDate; | ||
this.endDate = endDate; | ||
} | ||
|
||
private void validate(LocalDate startDate, LocalDate endDate) { | ||
if (startDate.isAfter(endDate)) { | ||
throw new IllegalArgumentException("시작일은 종료일보다 이전이어야 합니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package nextstep.sessions.domain; | ||
|
||
public enum SessionStatus { | ||
READY, RECRUITING, CLOSED | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package nextstep.sessions.domain; | ||
|
||
public enum SessionType { | ||
FREE, PAID | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package nextstep.enrollment.domain; | ||
|
||
import nextstep.payments.domain.Payment; | ||
import nextstep.sessions.domain.Session; | ||
import nextstep.sessions.domain.SessionStatus; | ||
import nextstep.users.domain.NsUser; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
|
||
import static org.assertj.core.api.Assertions.*; | ||
|
||
class EnrollmentTest { | ||
@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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ImageTypeTest를 제외하곤 성공 테스트의 경우 assertion이 없네요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @testrace 혹시 equals를 활용한 객체의 비교나 assertDoesNotThrow() 같은 메서드를 활용해야할까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 객체의 역할을 서로 다르게 해석했기에 생성만으로 검증이 충분하다고 판단하신 것 같아요 😃 다만, 학습 목표가 객체 설계와 구현이니 적절한 책임을 가진 도메인 객체를 도출해서 '기능'을 검증해 보시면 좋을 것 같습니다. Enrollment enrollment = new Enrollment();
Student student = enrollment.enroll(user, payment);
assertThat(student).equals(new Student(...)); 이벤트를 고려하셨다니 도메인간 결합을 최대한 끊으려 하신 것 같은데요. |
||
} | ||
|
||
@Test | ||
@DisplayName("결제 금액이 강의 수강료와 다르면 예외를 던진다.") | ||
void 결제_금액_강의_금액_불일치() { | ||
assertThatIllegalArgumentException().isThrownBy(() -> | ||
new Enrollment(new NsUser(), new Session(10_000L), new Payment("user_id", 0L, 0L, 20_000L))) | ||
.withMessage("결제 금액이 수강료와 일치하지 않습니다."); | ||
} | ||
|
||
@Test | ||
@DisplayName("강의 상태가 모집중이면 수강 신청이 가능하다.") | ||
void 모집중_강의_수강_신청() { | ||
new Enrollment(new NsUser(), new Session(SessionStatus.RECRUITING, 10_000L), new Payment("user_id", 0L, 0L, 10_000L)); | ||
} | ||
|
||
@Test | ||
@DisplayName("강의 상태가 모집중이 아니면 수강 신청 시 예외를 던진다.") | ||
void 모집중_아닌_강의_수강_신청() { | ||
assertThatIllegalArgumentException().isThrownBy(() -> | ||
new Enrollment(new NsUser(), new Session(SessionStatus.CLOSED, 10_000L), new Payment("user_id", 0L, 0L, 10_000L))) | ||
.withMessage("강의 상태가 모집중일 때만 수강 신청이 가능합니다."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package nextstep.sessions.domain; | ||
|
||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
|
||
import static org.assertj.core.api.Assertions.*; | ||
|
||
class EnrollmentCapacityTest { | ||
@Test | ||
@DisplayName("정원이 가득 찬 경우 인원 증가 시 예외를 던진다.") | ||
void 등록_인원_초과() { | ||
EnrollmentCapacity enrollmentCapacity = new EnrollmentCapacity(1, 1); | ||
assertThatThrownBy(enrollmentCapacity::increaseEnrollment) | ||
.isInstanceOf(IllegalStateException.class) | ||
.hasMessage("정원이 초과되었습니다."); | ||
} | ||
Comment on lines
+9
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요구사항에 따라 무료 강의는 정원 제한이 없기에 EnrollmentCapacity#increaseEnrollment 는 예외를 던지지 않아야 할 것 같아요. 더불어 예외 검증 시 assertThatThrownBy, assertThatIllegalArgumentException 가 혼용되고 있는데 |
||
|
||
@Test | ||
@DisplayName("정원이 가득 차지 않은 경우 등록 인원을 정상적으로 증가시킨다..") | ||
void 등록_인원_증가() { | ||
EnrollmentCapacity enrollmentCapacity = new EnrollmentCapacity(1, 0); | ||
enrollmentCapacity.increaseEnrollment(); | ||
} | ||
} |
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.
이 객체의 역할은 vaildator 에 가까운 것 같은데요.
student 변수는 활용되는 곳이 없어서 제거하면 어떨까 싶어요.
아직 미구현 단계라도 예상되는 코드는 제거하고 적은 역할만 부여해보셔도 좋을 것 같아요.
그리고 수강 신청이라는 기능은 Session의 기능이니 sessions 패키지 하위로 이동하면 어떨까요?
sessions와 동일한 레벨에 위치하신 이유가 궁금합니다 🤔
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.
@testrace
좋은 피드백 감사합니다!
말씀 주신 validator 역할이라는 관점도 공감이 가지만, 저는 Enrollment를 학생과 수업 간의 다대다 관계를 명시적으로 표현하는 도메인 객체로 보고 있습니다.
수강신청은 단순히 유효성 검사를 넘어서, "누가 어떤 수업에 얼마를 지불하고 신청했는가"라는 도메인 이벤트를 포착하고 있다고 생각했습니다.
패키지 구조에 대해서도, Enrollment가 단순히 Session의 하위 기능이라기보다는, 학생과 수업 사이의 독립된 관계이자 행위라고 보아 enrollment 패키지로 따로 분리했습니다.
혹시 수강신청에 대해서 제가 잘못이해하고 있다면 알려주시면 감사하겠습니다.
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.
수강신청 자체에 대해선 잘못이해하신 것은 없으신 것 같아요 😃
수강신청 미션은 이벤트 기반 방식은 고려하지 않았기 때문에 (사실 모든 미션이)
도메인 이벤트 객체라고 인지하지 않아서 Enrollment 의 역할을 단순히 validator 라고 생각했습니다.
이벤트를 위한 객체라면 클래스명을 EnrolledEvent 로 변경하면 어떨까 싶어요.
패키지 구조에서 말씀하신 �
학생과 수업 사이의 독립된 관계이자 행위
를 이해하지 못했는데요.그럼 session 하위 기능으로는 둘 수 없다는 의견이신지, 둘 다 가능하지만 독립적인 객체로 설계 하신건지 조금 더 자세하게 설명 부탁드려요.