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

dkswnkk · 2023-05-18T15:27:28Z

안녕하세요 멘토님!!, 2단계도 잘 부탁드립니다!!☺️

이번 2단계 미션은 요구사항이 그렇게까지 구체적이지 않아서 어디까지 설계해야 하나..라는 고민이 많이 들었네요

일단 2단계에서 요구하는 아래 사항들은 전부 구현했습니다.

과정(Course)은 기수 단위로 여러 개의 강의(Session)를 가질 수 있다.
강의는 시작일과 종료일을 가진다.
강의는 강의 커버 이미지 정보를 가진다.
강의는 무료 강의와 유료 강의로 나뉜다.
강의 상태는 준비중, 모집중, 종료 3가지 상태를 가진다.
강의 수강신청은 강의 상태가 모집중일 때만 가능하다.
강의는 강의 최대 수강 인원을 초과할 수 없다.

추가로 각 엔티티들의 컬럼들을 nullable = false하게 주고싶은데, 현재 JPA를 사용하는것이 아니고, 다음 단계에서 DDL로 정의하는 것 같아 null값을 검증하는 로직들은 추가하지 않았습니다!

nokchax

안녕하세요, 주형님!
두번째 단계 잘 진행해 주셨네요 :)
몇가지 코멘트 남겼는데 확인 부탁드립니다. 🙇

nokchax · 2023-05-19T12:49:02Z

README.md

+
+- DB 테이블 설계 없이 도메인 모델부터 구현한다.
+- 도메인 모델은 TDD로 구현한다.


커밋이력을 보니 TDD 방식이 아닌 구현후 테스트를 추가해 주셨더라구요,
진행하시면서 TDD도 시도해 보시는건 어떨까요?

이 부분은 제가 수시로 commit을 한 게 아니라 push 직전에 전부 커밋 하다 보니 순서가 어긋나서 그런 것 같습니다 ㅎㅎ..
혼란의 여지가 없도록 하겠습니당

nokchax · 2023-05-19T12:51:33Z

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

+
+import nextstep.courses.domain.enums.ImageType;
+
+public class Image extends BaseEntity {


image에 대해 어디까지 생각하시고 필드를 만들어 주셨는지 잘 모르겠지만, 필드에 대한 검증이 필요하지 않을까요?
예를 들어 사이즈에 대한 제한이라던지, 유효하지 않은 이미지 타입이 들어왔다 던지, uri가 유효한지 등 다양한 경우가 있을거 같네요

nokchax · 2023-05-19T12:53:54Z

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

+import java.util.ArrayList;
+import java.util.List;
+
+public class Session extends BaseEntity {


필드가 많아 보이네요, 시작일과 종료일을 하나로 묶은 클래스를 추출하거나,
users와 maximumEnrollment 를 묶어 추출하는 것처럼 연관이 있는 필드를 묶어 처리하면
Session 클래스가 가진 책임을 나눌 수 있을것 같아요

nokchax · 2023-05-19T12:54:44Z

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

+    }
+
+    private void checkSessionStatus() throws SessionEnrollmentException {
+        if (this.sessionStatus != SessionStatus.RECRUITING) {


값을 비교하지 말고 함수를 통해 판단하는 게 어떨까요?

sessionStatus.canEnroll()

nokchax · 2023-05-19T12:55:49Z

src/main/java/nextstep/courses/exception/InvalidSessionDateTimeException.java

@@ -0,0 +1,7 @@
+package nextstep.courses.exception;
+
+public class InvalidSessionDateTimeException extends Exception {


Exception을 상속하신 특별한 이유가 있을까요?

이 부분은 저도 고민을 좀 하다가 미션 기존 템플릿에 Exception을 상속하고 있길래 따라 맞춰서 작성했는데, 수정하도록 하겠습니다!

nokchax · 2023-05-19T12:57:17Z

src/test/java/nextstep/courses/domain/SessionTest.java

+
+import static org.junit.jupiter.api.Assertions.*;
+
+class SessionTest {


전반적으로 예외가 발생하는 경우에 대한 테스트만 존재하네요, 성공했을 때의 테스트도 해야하지 않을까요?
예를 들어 수강 신청을 했을 때, 수강생이 늘어난 것을 테스트 하는것이 있겠네요

nokchax · 2023-05-19T12:57:48Z

src/test/java/nextstep/courses/domain/SessionTest.java

+                        SessionType.FREE, SessionStatus.RECRUITING, 100));
+    }
+
+    @DisplayName("강의가 모집중이 아닌데 수강 등록을 하면 예외를 던진다.")


모집중이 아닌 경우에는 ENDED 외에도 존재하지 않나요?

nokchax

안녕하세요, 주형님!
피드백 잘 적용해 주셨습니다 👍
몇가지 좀 더 개선할 부분이 있어
코멘트 남겨뒀는데, 확인 부탁드려요 🙇

nokchax · 2023-05-20T06:16:54Z

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


 public class Image extends BaseEntity {
+    private static final Long MAX_SIZE = 5L * 1024 * 1024; // 5MB


nokchax · 2023-05-20T06:17:24Z

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

    }

    private void checkSessionStatus() throws SessionEnrollmentException {
-        if (this.sessionStatus != SessionStatus.RECRUITING) {
+        if (!SessionStatus.canEnroll(this.sessionStatus)) {


객체에게 너의 상태가 수강 신청 가능하니? 라고 물어보면 될 것 같은데요
this.sessionStatus.canEnroll() 이면 충분하지 않을까요?

nokchax · 2023-05-20T06:17:38Z

src/main/java/nextstep/courses/domain/enums/ImageType.java

@@ -14,4 +16,9 @@ public enum ImageType {
    public String getMimeType() {
        return this.value;
    }
+
+    public static boolean canUpload(ImageType type) {


ImageType 에는 등록 가능한 확장자 밖에 없기 때문에 무의미한 함수로 보입니다.
이미지의 확장자를 string으로 받아서 확인을 해야 의미가 생기지 않을까요?

nokchax · 2023-05-20T06:17:48Z

src/main/java/nextstep/courses/domain/enums/SessionStatus.java

@@ -14,4 +16,9 @@ public enum SessionStatus {
    public String getDescription() {
        return this.description;
    }
+
+    public static boolean canEnroll(SessionStatus sessionStatus) {


위에도 남겼지만 스태틱 메서드보다는 인스턴스 메서드를 통해 확인하는 게 객체지향 다운 방식이라고 봐야해요
추가적으로 enum의 함수도 테스트해주세요~!

nokchax · 2023-05-20T06:17:59Z

src/test/java/nextstep/courses/domain/SessionTest.java

+    private static final User SAMPLE_USER = new User(1L, "dkswnkk", "pwd", "안주형", "dkswnkk.dev@gmail.com");
+
+
+    @DisplayName("강의 시작일이 종료일보다 늦으면 예외를 던진다.")


SessionTime에 대한 테스트는 SessionTimeTest로 옮겨 코드를 나눠주세요

dkswnkk · 2023-05-20T14:55:35Z

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

+    }
+
+    private static void validateType(String typeString) throws InvalidImageException {
+        if (!ImageType.isValidType(typeString)) {


요 부분은 인스턴스 메서드의 사용이 불가능한데, 아직 생성할 떄의 검증 로직 같은 경우는 static 메서드를 활용해도 될까요..?

"인스턴스 메서드를 통해 확인하는 게 객체지향 다운 방식이다."라고 피드백을 주셨는데, 현재 이 로직 부분에서는 상태를 아직 가지기 않았기 때문에 static으로 검증해도 된다고 생각했습니다.! 아니면 설계 자체를 잘못한걸까용..?

validType(typeString); image.type = ImageType.valueOf(typeString.toUpperCase());

검증에 대한 책임이 올바른 곳에 존재하지 않아서 이렇게 된 것 같아요.
이미지 타입에 관한 검증은 ImageType enum 내부에 존재해야하지 않을까요?
valid함수를 제거하고. 아래와 같이 ImageType내부에 새로운 정적 함수를 만들어. (일종의 생성자)

image.type = ImageType.of(typeString);

를 통해 검증에 대한 책임을 ImageType에 전가하면 해결 되겠네요.

nokchax

안녕하세요, 주형님!
피드백 빠르게 잘 적용해 주셨네요 👍
한가지만 더 수정했으면 해서 코멘트 남겼습니다.
확인 부탁드려요 🙇

nokchax · 2023-05-21T06:30:13Z

src/main/java/nextstep/courses/domain/enums/SessionStatus.java

-    public static boolean canEnroll(SessionStatus sessionStatus) {
-        return Objects.equals(RECRUITING, sessionStatus);
+    public boolean canEnroll() {
+        return Objects.equals(RECRUITING, this);


this == RECRUITING

이면 충분 할 것 같네요

nokchax · 2023-05-21T06:35:57Z

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

+    }
+
+    private static void validateType(String typeString) throws InvalidImageException {
+        if (!ImageType.isValidType(typeString)) {


validType(typeString); image.type = ImageType.valueOf(typeString.toUpperCase());

검증에 대한 책임이 올바른 곳에 존재하지 않아서 이렇게 된 것 같아요.
이미지 타입에 관한 검증은 ImageType enum 내부에 존재해야하지 않을까요?
valid함수를 제거하고. 아래와 같이 ImageType내부에 새로운 정적 함수를 만들어. (일종의 생성자)

image.type = ImageType.of(typeString);

를 통해 검증에 대한 책임을 ImageType에 전가하면 해결 되겠네요.

nokchax · 2023-05-21T06:37:35Z

src/test/java/nextstep/courses/domain/enums/SessionStatusTest.java

+    @Test
+    @DisplayName("강의 상태가 모집중이 아닐때 false를 반환한다.")


@ParameterizedTest 를 통해 상태값을 파라미터로 받아 한줄이라도 중복을 줄여보면 어떨까요?

nokchax

안녕하세요, 주형님! 리뷰가 늦었네요 🙇
피드백 잘 적용해 주셨습니다 👍
바로 머지하겠습니다. 다음 단계 진행해주세요 :)

nokchax · 2023-05-22T11:10:49Z

src/main/java/nextstep/courses/domain/enums/ImageType.java

+        return Arrays.stream(values())
+                .filter(imageType -> imageType.getValue().equals(value.toLowerCase()))
+                .findFirst()
+                .orElseThrow(() -> new IllegalArgumentException("지원하지 않는 이미지 형식입니다."));


nokchax · 2023-05-22T11:11:09Z

src/test/java/nextstep/courses/domain/enums/SessionStatusTest.java

-                () -> assertFalse(SessionStatus.ENDED.canEnroll())
-        );
+    @ParameterizedTest
+    @EnumSource(value = SessionStatus.class, names = {"PREPARING", "ENDED"})


dkswnkk added 13 commits May 19, 2023 00:17

docs: 요구사항 작성

027da57

feat: 강의 타입 enum 추가

039f4f5

feat: 강의 상태 enum 추가

c841b61

feat: 강의 등록 Error 추가

1b387e7

feat: 강의 시간 검증 Error 추가

0ef7bcb

feat: 이미지 타입 enum 추가

473c987

feat: Image 엔티티 추가

bcb5360

feat: BaseEntity 추가

045326b

feat: 강의 Entity 추가

ef50c83

feat: 과정(Cousrse)가 여러개의 강의(Session) 가지도록 추가

bac2515

test: 테스트 코드 작성

5a15235

feat: Image 기본 생성자 추가

a363e07

style: 오타 수정

3fc152d

nokchax requested changes May 19, 2023

View reviewed changes

dkswnkk added 7 commits May 20, 2023 13:15

test: ImageTest 작성

da689aa

test: SessionTest 작성

89c8daa

refactor: Exception 상속 클래스 변경

8c01c7d

refactor: Session 책임 분리

e5d1fbe

feat: Image 검증 로직 추가

b93712f

reactor: 함수를 통해 ImageType 비교

bb2de82

reactor: 함수를 통해 SessionStatus 비교

55e9eb7

nokchax requested changes May 20, 2023

View reviewed changes

dkswnkk added 6 commits May 20, 2023 23:42

test: SessionStatus 테스트 코드 추가

874c900

test: ImageType 테스트 코드 추가

18df301

test: ImageTest 테스트 코드 수정

4bd121a

refactor: 이미지 확장자 String으로 받아서 검증하도록 변경

874521e

refactor: 에러 메시지 더 상세하게 작성

b477d2f

refactor: 인스턴스 메서드로 객체 상태 확인

d4607ef

dkswnkk commented May 20, 2023

View reviewed changes

nokchax requested changes May 21, 2023

View reviewed changes

dkswnkk added 3 commits May 21, 2023 17:06

refactor: @ParameterizedTest 사용

dbec538

refactor: 간단하게 비교하도록 변경

dcc6a76

refactor: ImageType 책임 ImageType이 가지도록 변경

979be45

nokchax approved these changes May 22, 2023

View reviewed changes

nokchax merged commit 87b58fe into next-step:dkswnkk May 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dkswnkk commented May 18, 2023 •

edited

nokchax left a comment

nokchax May 19, 2023

dkswnkk May 19, 2023

nokchax May 19, 2023

nokchax May 19, 2023

nokchax May 19, 2023

nokchax May 19, 2023

dkswnkk May 19, 2023

nokchax May 19, 2023

nokchax May 19, 2023

nokchax left a comment

nokchax May 20, 2023

nokchax May 20, 2023

nokchax May 20, 2023

nokchax May 20, 2023

nokchax May 20, 2023

dkswnkk May 20, 2023

nokchax May 21, 2023

nokchax left a comment

nokchax May 21, 2023

nokchax May 21, 2023

nokchax May 21, 2023

nokchax left a comment

nokchax May 22, 2023

nokchax May 22, 2023


		- DB 테이블 설계 없이 도메인 모델부터 구현한다.
		- 도메인 모델은 TDD로 구현한다.


		import nextstep.courses.domain.enums.ImageType;

		public class Image extends BaseEntity {

		@@ -0,0 +1,7 @@
		package nextstep.courses.exception;

		public class InvalidSessionDateTimeException extends Exception {


		import static org.junit.jupiter.api.Assertions.*;

		class SessionTest {


		public class Image extends BaseEntity {
		private static final Long MAX_SIZE = 5L * 1024 * 1024; // 5MB

		private static final User SAMPLE_USER = new User(1L, "dkswnkk", "pwd", "안주형", "dkswnkk.dev@gmail.com");


		@DisplayName("강의 시작일이 종료일보다 늦으면 예외를 던진다.")

		@Test
		@DisplayName("강의 상태가 모집중이 아닐때 false를 반환한다.")

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

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

Conversation

dkswnkk commented May 18, 2023 • edited

nokchax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nokchax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nokchax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nokchax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkswnkk commented May 18, 2023 •

edited