🚀 1단계 - 리팩터링(상품) by nomoreFt · Pull Request #356 · next-step/ddd-tactical-design

nomoreFt · 2025-02-25T13:42:07Z

안녕하세요. 멘토님. 과제 제출합니다.

ProductName이 비속어 검증이 꼭 필요하다는 점을 내부 생성할 때 표현하고 싶은데, ProfanityChecker를 static factory 생성자에서 의존하는게 맞을지 고민이 되네요.

…Connection 취득 지연

Rok93

현우님 안녕하세요. 리팩터링 미션을 함께하게된 김경록입니다. 🙇‍♂️
1단계 미션 잘 진행해주셨네요. 👍👍
domain 쪽은 꼼꼼하게 잘 진행해주신 것 같은데,
추가적으로 service 혹은 controller 쪽에 대한 수정도 같이 진행해주시면 좋을 것 같아요. 😉

그 외에도 몇몇 코멘트 남겨두었는데 확인 부탁드릴게요. 🙏

Rok93 · 2025-02-25T14:55:04Z

docker-compose.yml

@@ -0,0 +1,20 @@
+version: '3.8'


docker-compose 파일이 docker 디렉토리에 이미 있는걸로 알고있는데, docker-compose file을 밖에 따로 두신 이유가 있으신가요? 👀

아 있는줄 몰랐네요 ㅎㅎ 삭제했습니다.

Rok93 · 2025-02-26T11:54:29Z

src/main/java/kitchenpos/products/application/ProductService.java

        }
        final String name = request.getName();
-        if (Objects.isNull(name) || purgomalumClient.containsProfanity(name)) {
+        if (Objects.isNull(name) || profanityChecker.containsProfanity(name)) {


ProductName.create 메서드를 통해서 ProductName을 생성할 수 있고,
해당 정적 팩토리 메서드에서 생성할 때, 필요한 유효성 검증들을 모두 해주고있는데요.

구현해주신 create 메서드로 ProductName 객체를 생성하도록 변경해보면 어떨까요? 😃

ProductNamePolicy로 아예 분리했습니다.

Rok93 · 2025-02-26T11:56:16Z

src/main/java/kitchenpos/products/application/ProductService.java

+        if (Objects.isNull(name) || profanityChecker.containsProfanity(name)) {
            throw new IllegalArgumentException();
        }
        final Product product = new Product();


또한 Product 객체를 생성하는 방법도 지금처럼 객체를 생성한 뒤, setter를 통해 상태 값을 초기화해주는 방식 대신에,
생성자 혹은 정적 팩토리 메서드를 통해서 객체를 생성하도록 변경해주시면 좋을 것 같아요. 😉

Rok93 · 2025-02-26T11:58:15Z

src/main/java/kitchenpos/products/tobe/domain/support/UUIDBasedProductIdGenerator.java

@@ -0,0 +1,15 @@
+package kitchenpos.products.tobe.domain.support;


ProductIdGenerator의 구현체인 UUIDBasedProductIdGenerator는 support 패키지에 위치시키신 이유가 있으실까요?
의도가 궁금해서 질문드립니다 : )

UUID, ProductNamePolicy 와 같이 domain에 아주 밀접한 구현체 자체는 내부에 위치시키려 했습니다. 단순 패키징에 대한 구분입니다.

Rok93 · 2025-02-26T12:01:18Z

src/main/java/kitchenpos/shared/domain/DomainEntity.java

+        if (other == null) {
+            return false;
+        }
+
+        if (getId() == null) {
+            return false;
+        }


사소하지만 아래처럼 변경해볼 수도 있을 것 같네요.

Suggested change

if (other == null) {

return false;

}

if (getId() == null) {

return false;

}

if (other == null || getId() == null) {

return false;

}

Rok93 · 2025-02-26T12:02:54Z

src/main/java/kitchenpos/shared/domain/DomainEntity.java

+
+import java.io.Serializable;
+
+public abstract class DomainEntity<T extends DomainEntity<T, TID>, TID> implements Serializable {


DomainEntity와 ValueObject에서 공통되는 요소를 추상 클래스로 뽑아내셨군요? 👍💯

AggregateRoot도 추가했습니다.

Rok93 · 2025-02-26T12:04:48Z

src/main/java/kitchenpos/shared/domain/Money.java

+        return new Money(BigDecimal.valueOf(amount));
+    }
+
+    public static Money wons(double amount) {


이 메서드는 사용하지 않는 것 같은데, 사용하지 않는 메서드는 과감히 제거해주시면 좋을 것 같아요. 😃

마찬 가지로 subtract, isGreaterThanOrEqual 등도 지금은 사용처가 없는 것 같은데요.

나중에 분명 필요할꺼야...! 라는 생각이드실 수도 있지만, 저는 개인적으로 필요한 시점에 만드는게 더 좋다고 생각하는 편입니다. 😉

소프트웨어 개발 3대원칙 중... YAGNI

넵 usage 없는 부분들은 미리미리 생성하지 않겠습니다.

Rok93 · 2025-02-26T12:07:57Z

src/main/java/kitchenpos/shared/domain/Money.java

+
+    private Money(BigDecimal amount) {
+        if(amount == null || amount.compareTo(BigDecimal.ZERO) < 0) {
+            throw new IllegalArgumentException("금액은 null 이거나 0보다 작을 수 없습니다.");


얼마의 amount가 메서드 파라미터로 들어왔는지, 예외 메시지로 보여준다면 조금 더 에러를 추적하기가 수월해지지 않을까요? 😃

Rok93 · 2025-02-26T12:18:41Z

src/main/java/kitchenpos/products/application/ProductService.java

        }
        final String name = request.getName();
-        if (Objects.isNull(name) || purgomalumClient.containsProfanity(name)) {
+        if (Objects.isNull(name) || profanityChecker.containsProfanity(name)) {


product context에서 제공하는 API들에 대해서 요청 혹은 응답 값으로 DomainEntity를 그대로 쓰고있는 것 같은데, DTO로 변경해보면 어떨까요? 👀

Rok93 · 2025-02-26T12:29:14Z

src/main/java/kitchenpos/products/tobe/domain/ProductName.java

+        this.name = name;
+    }
+
+    public static ProductName create(ProfanityChecker profanityChecker, String name) {


(질문) ProductName이 비속어 검증이 꼭 필요하다는 점을 내부 생성할 때 표현하고 싶은데, ProfanityChecker를 static factory 생성자에서 의존하는게 맞을지 고민이 되네요.

ProductName 객체를 생성하는데있어 ProfanityChecker이 직접적으로 관여하기 때문이 아닐까 싶어요.
ProductName 생성 정책을 검증해주는 역할을 도메인 서비스 객체(ex. ProdcutNamePolicy)에게 위임해볼 수 있을 것 같아요.

nomoreFt · 2025-03-03T13:05:18Z

리팩토링하다가 조금 많이 간 건 아닌가 고민이 되네요 하하..

shared로 공통 Money,Quantity같은 밸류오브젝트를 선언했습니다.
도메인 객체를 풍성하게 수정하고, 비즈니스 로직을 내부에 넣었습니다.
밸류오브젝트나 엔티티가 생성시에 검증되어 service 내부에서 DTO 로도 쓰이게 하고, 이에 따라 service에서 중복으로 테스트하는 건들은 disabled 처리 해봤습니다.
요구사항을 usecase로 분리했습니다.
분리된 요구사항별로 command와 query로 나누어 구현해봤습니다.
aggregateRoot 타입을 선언해서 registerEvent로 Product의 값 변화에 따라 Menu의 display 여부를 결정하는 로직의 결합도를 줄여봤습니다.

이 과정에서 테스트하기 위해 Product의 발행된 이벤트 리스트를 가져오는 메서드를 public으로 오버라이딩 했는데, 어떻게 생각하실지 궁금합니다.

nomoreFt added 6 commits February 25, 2025 17:15

chore: DB Connection 설정 변경 – HikariCP, Hibernate Auto-Commit 비활성화로 Db…

1ce635f

…Connection 취득 지연

chore: docker compose 추가

7f3a747

feat(core): 기본 Entity Type 추가

28bb200

feat(core): 공통 Money 타입, Converter 추가

6626e1a

refactor(product): Product 리팩토링

54e1cce

refactor(product): Product 리팩토링

ef2b0b9

Rok93 requested changes Feb 26, 2025

View reviewed changes

nomoreFt added 7 commits February 27, 2025 21:43

chore: 중복 docker compose 삭제

4696cff

refactor: Product refactoring

5bd913b

refactor(product): Service 개선 진행

956a3f4

refactor(product): 기존 도메인 리팩토링 + 관련된 사이드 이펙트 수정 및 테스트 정상 작동

4061ecb

refactor(product): 기존 도메인 리팩토링 + 관련된 사이드 이펙트 수정 및 테스트 정상 작동

893b1e9

chore(product): flyway 설정

a8f25d6

feat(product): 공통 AggregateRoot 타입 선언

5d63264

refactor(product): ValueObject 자잘한 수정

cae28ff

		@@ -0,0 +1,15 @@
		package kitchenpos.products.tobe.domain.support;


		import java.io.Serializable;

		public abstract class DomainEntity<T extends DomainEntity<T, TID>, TID> implements Serializable {

Conversation

nomoreFt commented Feb 25, 2025

Uh oh!

Rok93 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!

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!

nomoreFt commented Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nomoreFt commented Mar 3, 2025 •

edited

Loading