2단계 - 수강신청(도메인 모델) #728

heewonham · 2025-04-15T12:17:27Z

수강신청

과정(Course)은 기수 단위로 운영하며, 여러 개의 강의(Session)를 가질 수 있다.

session

강의는 시작일과 종료일을 가진다.
강의는 무료 강의와 유료 강의로 나뉜다.
- 무료 강의는 최대 수강 인원 제한이 없다.
- 유료 강의는 강의 최대 수강 인원을 초과할 수 없다.
- 유료 강의는 수강생이 결제한 금액과 수강료가 일치할 때 수강 신청이 가능하다.
강의 상태는 준비중, 모집중, 종료 3가지 상태를 가진다.
강의 수강신청은 강의 상태가 모집중일 때만 가능하다.
유료 강의의 경우 결제는 이미 완료한 것으로 가정하고 이후 과정을 구현한다.
- 결제를 완료한 결제 정보는 payments 모듈을 통해 관리되며, 결제 정보는 Payment 객체에 담겨 반한된다.

image

강의는 강의 커버 이미지 정보를 가진다.
- 이미지 크기는 1MB 이하여야 한다.
- 이미지 타입은 gif, jpg(jpeg 포함),, png, svg만 허용한다.
- 이미지의 width는 300픽셀, height는 200픽셀 이상이어야 하며, width와 height의 비율은 3:2여야 한다.

nooose

희원님 안녕하세요
도메인 모델들 잘 구현해 주셨어요!
더 개선했으면 하는 부분 코멘트로 남겨놨으니
확인 후 재요청 부탁드립니다!

nooose · 2025-04-15T13:28:51Z

src/main/java/nextstep/payments/domain/Payment.java

@@ -26,4 +26,16 @@ public Payment(String id, Long sessionId, Long nsUserId, Long amount) {
        this.amount = amount;
        this.createdAt = LocalDateTime.now();
    }
+
+    public void isValidatePayment(Long sessionId, Long nsUserId, Long amount) {
+        if (this.sessionId != sessionId) {


Long.valueOf(1000) == Long.valueOf(1000)

위 결과는 어떤 값이 나올까요? == 비교가 맞을까요?

nooose · 2025-04-15T13:42:24Z

src/main/java/nextstep/courses/domain/PaidSession.java

+        if (payment == null) {
+            throw new IllegalArgumentException("결제 정보가 없습니다.");
+        }
+        payment.isValidatePayment(id, studentId, tuitionFee);


payment.isValidatePayment(sessionId, studentId, tuitionFee)는 결제 객체가 수강신청의 유효성을 판단하는 책임까지 떠맡고 있네요

Payment는 결제 도메인에 속하는 객체이기 때문에 책임을 분리하면 좋겠습니다.

수강신청의 유효성 판단은 Session의 책임이므로
결제 정보가 수강신청 요건을 만족하는지 여부는 Session의 관점에서 판단하고 Payment는 그 판단에 필요한 정보만 제공하면 충분하다고 생각합니다.

따라서 validateRegistration 메소드 내에서 Payment 정보를 참조해 체크하는 방식으로 변경해 보면 좋을 것 같아요

nooose · 2025-04-15T13:46:09Z

src/main/java/nextstep/courses/domain/Session.java

+    public boolean isRegistered(Long studentId) {
+        return registeredStudents.contains(studentId);
+    }


이 부분은 테스트 코드를 위한 메소드로 보이네요

registeredStudents 필드를 게터로 꺼내와서 테스트했어도 되지 않았을까요?

@nooose getter 함수보다는 각 함수를 만들어서사용하는것이 좋다는 강의 내용을 보고 만들었습니다.
getter가 더 나을까요?

비즈니스 로직을 위한 것이 아닌 테스트 목적의 public 메서드로 보여서 드린 리뷰였습니다.
만약 테스트만을 위한 코드가 많아질수록 도메인 모델 내 코드가 많아져 복잡도가 높아질 수 있어서요

위와 같은 간단한 검사라면 게터를 사용해서도 충분히 검증할 수 있다고 생각합니다.
정답은 없으므로 참고해 주세요!

nooose · 2025-04-15T13:46:55Z

src/main/java/nextstep/courses/domain/Session.java

+    protected Set<Long> registeredStudents = new HashSet<>();
+    public Session(Long id, String name, LocalDate startDate, LocalDate endDate, Image coverImage, LectureStatus status) {


Suggested change

protected Set<Long> registeredStudents = new HashSet<>();

public Session(Long id, String name, LocalDate startDate, LocalDate endDate, Image coverImage, LectureStatus status) {

protected Set<Long> registeredStudents = new HashSet<>();

public Session(Long id, String name, LocalDate startDate, LocalDate endDate, Image coverImage, LectureStatus status) {

멤버변수와 생성자는 개행으로 구분짓는 것이 컨벤션입니다 😄

nooose · 2025-04-15T13:50:24Z

src/main/java/nextstep/courses/domain/PaidSession.java

+public class PaidSession extends Session {
+    private int maxCapacity;
+    private Long tuitionFee;


Suggested change

public class PaidSession extends Session {

private int maxCapacity;

private Long tuitionFee;

public class PaidSession extends Session {

private int maxCapacity;

private int tuitionFee;

유료강의이므로 수강료가 null일 수 없으니 래퍼클래스 대신 primitive 타입을 사용하는 것이 좋겠네요
또한 수강료가 long 타입만큼 필요한 범위인지도 고려해보면 좋겠습니다.
int 양수 표현 범위도 충분하다고 생각해요!

nooose · 2025-04-15T13:55:30Z

src/main/java/nextstep/courses/domain/Image.java

+    }
+
+    private void validate() {
+        if (sizeInBytes > 1024 * 1024) {


1024 * 1024 이 값도 매직넘버에 해당되네요!

nooose · 2025-04-15T13:56:30Z

src/main/java/nextstep/courses/domain/Image.java

+        this.fileName = fileName;
+        this.contentType = contentType;
+        this.sizeInBytes = sizeInBytes;
+        this.width = width;
+        this.height = height;
+        validate();


Suggested change

this.fileName = fileName;

this.contentType = contentType;

this.sizeInBytes = sizeInBytes;

this.width = width;

this.height = height;

validate();

validate();

this.fileName = fileName;

this.contentType = contentType;

this.sizeInBytes = sizeInBytes;

this.width = width;

this.height = height;

결과적으로는 상관없지만 어차피 검증을 해야한다면 먼저 검증하는것이 맞지 않을까요?

nooose · 2025-04-15T14:03:03Z

src/main/java/nextstep/courses/domain/Session.java

+    protected LocalDate startDate;
+    protected LocalDate endDate;


endDate가 startDate보다 빠른 시간이거나 같을 때도 검증해 보면 좋겠습니다
기간이라는 값 객체로 만들어봐도 좋겠구요 😄

nooose · 2025-04-15T14:15:59Z

src/main/java/nextstep/courses/domain/Course.java

@@ -12,6 +12,8 @@ public class Course {
    private LocalDateTime createdAt;

    private LocalDateTime updatedAt;
+    private int generation;
+    private Sessions sessions;

    public Course() {


요구사항에는 딱히 없지만
과정(Course)에 강의를 추가 삭제하는 유즈케이스도 고려해 보면 좋겠네요
일반적인 상황같아서요!

nooose · 2025-04-15T14:17:54Z

src/main/java/nextstep/courses/domain/LectureType.java

@@ -0,0 +1,6 @@
+package nextstep.courses.domain;
+
+public enum LectureType {


Session과 Lecture는 어떤 차이가 있나요?
차이가 없다면 통일하는게 일관성있는 것 같다고 생각합니다!

nooose

희원님 리뷰가 늦어서 죄송합니다.
코멘트 남겨놨으니 다음 테이블 매핑 단계 넘어가기전에 확인 부탁드려요
고민있으시면 DM 또는 코멘트 남겨주세요!

nooose · 2025-04-17T15:22:19Z

src/main/java/nextstep/courses/domain/Session.java

+    public boolean isRegistered(Long studentId) {
+        return registeredStudents.contains(studentId);
+    }


비즈니스 로직을 위한 것이 아닌 테스트 목적의 public 메서드로 보여서 드린 리뷰였습니다.
만약 테스트만을 위한 코드가 많아질수록 도메인 모델 내 코드가 많아져 복잡도가 높아질 수 있어서요

위와 같은 간단한 검사라면 게터를 사용해서도 충분히 검증할 수 있다고 생각합니다.
정답은 없으므로 참고해 주세요!

nooose · 2025-04-17T15:24:06Z

src/main/java/nextstep/courses/domain/Image.java

+    private String fileName;
+    private String contentType;
+    private long sizeInBytes;
+    private int width;
+    private int height;
+    private static final int IMAGE_SIZE_1MB = 1024 * 1024;
+    private static final int WIDTH_LIMIT = 300;
+    private static final int HEIGHT_LIMIT = 200;
+    private static final double REQUIRED_ASPECT_RATIO = 1.5;
+    private static final double ASPECT_RATIO_TOLERANCE = 0.01;


Suggested change

private String fileName;

private String contentType;

private long sizeInBytes;

private int width;

private int height;

private static final int IMAGE_SIZE_1MB = 1024 * 1024;

private static final int WIDTH_LIMIT = 300;

private static final int HEIGHT_LIMIT = 200;

private static final double REQUIRED_ASPECT_RATIO = 1.5;

private static final double ASPECT_RATIO_TOLERANCE = 0.01;

private static final int IMAGE_SIZE_1MB = 1024 * 1024;

private static final int WIDTH_LIMIT = 300;

private static final int HEIGHT_LIMIT = 200;

private static final double REQUIRED_ASPECT_RATIO = 1.5;

private static final double ASPECT_RATIO_TOLERANCE = 0.01;

private String fileName;

private String contentType;

private long sizeInBytes;

private int width;

private int height;

관례에 따라 상수는 멤버변수보다 먼저 배치하는데
아래에 배치하는 스타일이신가요?

nooose · 2025-04-17T15:25:09Z

src/main/java/nextstep/courses/domain/Image.java

+    private long sizeInBytes;
+    private int width;
+    private int height;
+    private static final int IMAGE_SIZE_1MB = 1024 * 1024;


Suggested change

private static final int IMAGE_SIZE_1MB = 1024 * 1024;

private static final int MAX_IMAGE_SIZE = 1024 * 1024;

1024 * 1024 값이 1MB라는 것은 중요하지 않다고 생각해요
이 값이 무슨 의미를 가지는지 나타내는 것이 더 좋다고 느껴집니다!

nooose · 2025-04-17T15:27:14Z

src/main/java/nextstep/courses/domain/Session.java

+    protected Period period;
+    protected Image coverImage;
+    protected SessionStatus status;
+    protected Set<Long> registeredStudents = new HashSet<>();


앞으로 도메인 모델과 테이블간 매핑을 해야할텐데
수강생이라는 도메인 모델을 만들어보면 좋을 것 같아요

단순 Long 숫자값이 무엇을 의미하는지 표현하기가 어려울 것 같네요!

nooose · 2025-04-17T15:28:50Z

src/main/java/nextstep/courses/domain/Capacity.java

@@ -0,0 +1,20 @@
+package nextstep.courses.domain;
+
+public class Capacity {


값 객체 클래스네요 👍
동등성 보장을 위해 equals 와 hashCode도 함께 구현해 보면 좋겠습니다.
이 클래스 뿐만 아니라 다른 값 객체에도 반영하면 좋겠습니다 😄

nooose · 2025-04-17T15:30:12Z

src/main/java/nextstep/courses/domain/PaidSession.java

+        if (!studentId.equals(payment.getNsUserId())) {
+            throw new IllegalArgumentException("결제한 사용자와 일치하지 않습니다.");
+        }
+        if (this.tuitionFee.getAmount() != payment.getAmount()) {


게터 사용대신 tuitionFee에게 메시지를 보내보면 어떨까요?

메소드 네이밍은 참고만 해주세요~

Suggested change

if (this.tuitionFee.getAmount() != payment.getAmount()) {

if (this.tuitionFee.isSameAmount(payment.getAmount())) {

heewonham added 5 commits April 15, 2025 21:11

feat : image 도메인 생성 및 테스트 코드 추가

68f8e60

feat : lecture type 및 lecture status enum 추가

198ab4c

feat : session 추상 클래스 추가 및 유료, 무료 세션 클래스 추가 및 테스트 코드 추가

927eb4f

feat : course 기수 단위 운영 추가, 세션 일급컬렉션 추가

413fb4b

feat : readme 추가

1df7c4f

nooose requested changes Apr 15, 2025

View reviewed changes

heewonham added 4 commits April 17, 2025 11:19

refactor : (reivew) Lecturexxx -> Sessionxxx enum 이름 변경

767b761

refactor : (review) capacity, tuitionFee 값 객체추출

a4837f4

refactor : (review) period 객체 추출

8ffdca6

refactor : (review) 상수, 세션 제거/추가 기능, register 시 payment 유효성 session에서처리

03942b2

nooose approved these changes Apr 17, 2025

View reviewed changes

nooose merged commit cb8f94f into next-step:heewonham Apr 17, 2025

heewonham mentioned this pull request Apr 21, 2025

3단계 - 수강신청(DB 적용) #769

Open

		protected Set<Long> registeredStudents = new HashSet<>();
		public Session(Long id, String name, LocalDate startDate, LocalDate endDate, Image coverImage, LectureStatus status) {

		@@ -0,0 +1,6 @@
		package nextstep.courses.domain;

		public enum LectureType {

	private static final int IMAGE_SIZE_1MB = 1024 * 1024;
	private static final int MAX_IMAGE_SIZE = 1024 * 1024;

		@@ -0,0 +1,20 @@
		package nextstep.courses.domain;

		public class Capacity {

	if (this.tuitionFee.getAmount() != payment.getAmount()) {
	if (this.tuitionFee.isSameAmount(payment.getAmount())) {

2단계 - 수강신청(도메인 모델) #728

2단계 - 수강신청(도메인 모델) #728

Uh oh!

Conversation

heewonham commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

수강신청

Uh oh!

nooose left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nooose left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

heewonham commented Apr 15, 2025 •

edited

Loading