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

wlwpfh · 2025-04-13T03:22:25Z

안녕하세요. 이번 단계 리뷰 잘부탁드립니다.

감사합니다!

wooobo

안녕하세요!
2단계도 잘진행해주셨습니다 😄

몇가지 코멘트 남겨놓았어요~
확인 후 재요청 부탁드립니다!

그럼 화이팅입니다!

wooobo · 2025-04-13T08:17:35Z

src/main/java/nextstep/courses/domain/session/Enrollment.java

+        this.sessionCapacity = sessionCapacity;
+    }
+
+    public Enrollment(SessionCapacity sessionCapacity) {


PR요청시 사용하지 않는 메소드는 과감히 제거하는것이 좋다고생각합니다.
혹시 추후 참고해야되는 코드라면, 적절한 commit메시지로 남겨 놓는것도 하나의 방법일것 같습니다 🤔

wooobo · 2025-04-13T08:38:20Z

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

+    }
+
+    private boolean canApply(LocalDate enrollDate, Payment payment) {
+        if (!state.canRecruit()) {


Suggested change

if (!state.canRecruit()) {

if (state.cannotRecruit()) {

!표현 보다는 있는 그대로 메소드명으로 표현하는게 가독성에 좋을수도 있다고생각되네요 🤔

wooobo · 2025-04-13T08:43:37Z

src/main/java/nextstep/courses/domain/session/SessionCapacity.java

+    public void increase() {
+        capacity++;
+        isValidCapacity();
+    }


Suggested change

public void increase() {

capacity++;

isValidCapacity();

}

public void increase() {

capacity++;

isValidCapacity();

}

해당 코드를 보고 의문점이 드는게 한가지 있는데요,

try { SessionCapacity sessionCapacity = new SessionCapacity(10, 5); sessionCapacity.increase(); } catch (CannotEnrollException e) { System.out.println(e.getMessage()); } // blabla

위 테스트 코드는 10어야 할까요 11이어야할까요?

그렇다면 무엇이 문제일까요?

public void increase() { isValidCapacity(); capacity++; }

위치를 조정해보거나, capacity 를 불변객체로 관점 전환 후 해결해볼수도 있을겄같네요 🤔

wooobo · 2025-04-13T12:03:18Z

src/main/java/nextstep/courses/domain/session/SessionImage.java

+
+    private void checkValidSessionImage() {
+        if (this.width < MIN_WIDTH || this.height < MIN_HEIGHT) {
+            throw new InvalidImageException("이미지의 width는 300픽셀, height는 200픽셀 이상이어야 한다.");


조건이 변경될때마다 예외 메시지까지 함께 변경해야하는것은 유지보수시 리스크가 있지 않을까요?

wooobo · 2025-04-13T12:11:01Z

src/main/java/nextstep/courses/InvalidImageException.java

@@ -0,0 +1,11 @@
+package nextstep.courses;
+
+public class InvalidImageException extends RuntimeException {


단순히 메시지만 받는것이라면,
java 기본 excpetion으로 사용할수 있었을 텐데,
custom exception을 만든 이유가 무엇인지 궁금합니다!

현재 도메인별로 custom exception으로 예외처리를 하여 이와 같게 진행하였습니다..!

wooobo · 2025-04-13T12:46:03Z

src/main/java/nextstep/courses/domain/session/SessionDate.java

+        this.startDate = startDate;
+        this.endDate = endDate;


endDate가 startDate보다 이전이면 객체가 생성안되게 하는게 더 확실하지 않을까요? 아니면, endDate가 startDate보다 이전이라도 생성되어야하는 경우가 있나요?

wooobo · 2025-04-13T12:55:41Z

src/main/java/nextstep/courses/strategy/PaidPaymentStrategy.java

+        if (payment.isSameAmount(price))
+            return true;
+        return false;


Suggested change

if (payment.isSameAmount(price))

return true;

return false;

return payment.isSameAmount(price);

쉽게 표현할수있는것은 쉽게 표현해도 좋을것 같아요~

wooobo · 2025-04-13T12:58:10Z

src/test/java/nextstep/courses/strategy/PaymentStrategyTest.java

+        FreePaymentStrategy freePaymentStrategy = new FreePaymentStrategy();
+        assertTrue(freePaymentStrategy.payable(new Payment()));
+    }
+


무료일경우 결제금액이 있는 Payment가 입력되어도 수행되는게 안전할까요?

@Test public void free2(){ FreePaymentStrategy freePaymentStrategy = new FreePaymentStrategy(); assertTrue(freePaymentStrategy.payable(new Payment("1", 1L, 1L, 1000L))); }

FreePaymentStrategy에서는 모두 true를 반환하기 때문에 괜찮다고 생각하였습니다!

wooobo · 2025-04-13T13:22:12Z

README.md

+  - [x] 강의는 강의 커버 이미지 정보를 가진다.
+  - [ ] 강의는 무료 강의와 유료 강의로 나뉜다.
+    - [ ] 무료 강의는 최대 수강 인원 제한이 없다.


해당 요구사항이 구현되지 않은이유가 있을까요?

wlwpfh added 15 commits April 10, 2025 19:40

[docs] 기능요구사항 정리

67d8c98

[feat] 강의 이미지 타입 설정

69f7da0

[feat] 강의 이미지 클래스 구현

eafe229

[feat] 강의 수강 인원 구현

b981844

[feat] 강의 상태 설정

101a978

[feat] 강의 날짜 설정

c26c840

[feat] 유료 / 무료 전략 패턴 설정

62b1673

[refactor] 패키지 이동

eb4aecd

[feat] 등록 행위 설정

1acc6f1

[feat] 결제 방식 수정 및 테스트 추가

eeeae90

[feat] 강의 구현 설정

5a0bb9f

[refactor] 미사용 함수 제거

d8f480f

[refactor] 정적 팩토리 메소드 추가

f249b5d

[refactor] 질문 삭제 시 validation 추가

1b7b8ab

[refactor] 이미지, 등록 시 예외 추가

460de6b

wooobo requested changes Apr 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

wlwpfh commented Apr 13, 2025

wooobo left a comment

wooobo Apr 13, 2025

wooobo Apr 13, 2025

wooobo Apr 13, 2025

wooobo Apr 13, 2025

wooobo Apr 13, 2025

wlwpfh Apr 14, 2025

wooobo Apr 13, 2025

wooobo Apr 13, 2025

wooobo Apr 13, 2025

wlwpfh Apr 15, 2025

wooobo Apr 13, 2025

		@@ -0,0 +1,11 @@
		package nextstep.courses;

		public class InvalidImageException extends RuntimeException {

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

Are you sure you want to change the base?

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

Conversation

wlwpfh commented Apr 13, 2025

wooobo 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment