Skip to content
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

3단계 - 기능 우선 패키지 구성하기 #308

Open
wants to merge 1 commit into
base: laonzenamoon
Choose a base branch
from

Conversation

LaOnZenaMoon
Copy link

이번에도 잘 부탁드립니다 🙏

Copy link
Member

@Hyeon9mak Hyeon9mak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 이준님, 기능 패키지 구성 잘 진행해주셨습니다.
용어사전과 모델링을 정의하면서 고민했던 컨텍스트가 코드에도 반영되면 진짜 DDD 라고 부를 수 있을거 같아요.
관련해서 몇 가지 코멘트 남겼습니다.

Comment on lines +1 to +5
package kitchenpos.menu.application;

import kitchenpos.domain.*;
import kitchenpos.infra.PurgomalumClient;
import kitchenpos.common.infra.PurgomalumClient;
import kitchenpos.menu.domain.Menu;
import kitchenpos.menu.domain.MenuGroup;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Menu Context 하위에 Menu, MenuGroup 두가지 Aggregate Root 가 존재하는 것 같아요.
Context 를 기반으로 분리할 때와 Aggregate Root 를 기반으로 분리할 때의 장단점을 고민해보면 좋을 것 같습니다.

Copy link
Author

@LaOnZenaMoon LaOnZenaMoon Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

곰곰히 생각을 해보았는데, Aggregate Root 를 기반으로 분리하는 게 낫겠다는 생각이 드네요 🤔

그렇게 하지 않으면 나중에, 서로 다른 Aggregate Root 의 하위 엔티티 간에 직접 참조가 발생할 수도 있을 것 같네요.
우선 Aggregate Root 기반으로 분리를 해보겠습니다.

혹시 Context 기반으로 분리했을 때는 어떤 장단점이 있을 수 있을까요? 🤔🤔

Comment on lines +6 to +12
import kitchenpos.order.domain.Order;
import kitchenpos.order.domain.OrderLineItem;
import kitchenpos.order.domain.OrderRepository;
import kitchenpos.order.domain.OrderStatus;
import kitchenpos.order.domain.OrderTable;
import kitchenpos.order.domain.OrderTableRepository;
import kitchenpos.order.domain.OrderType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order 와 관련된 모든 정보에서 Order 접두어를 사용하는데, 이 부분에 대한 이준님만의 전략이나 생각이 궁금합니다 😄

Comment on lines +1 to +7
package kitchenpos.order.application;

import kitchenpos.domain.*;
import kitchenpos.infra.KitchenridersClient;
import kitchenpos.order.infra.KitchenridersClient;
import kitchenpos.menu.domain.Menu;
import kitchenpos.menu.domain.MenuRepository;
import kitchenpos.order.domain.Order;
import kitchenpos.order.domain.OrderLineItem;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 Order 에서 Menu 를 직접 의존하는 것에 대해 어떻게 생각하실까요?

Copy link
Author

@LaOnZenaMoon LaOnZenaMoon Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

복잡도가 높아지고, 다른 Aggregate 를 통해 수정하는 문제가 발생할 수 있을 것 같아서,
서로 다른 Aggregate 간에 직접 참조는 좋지 않을 것 같습니다. 🤔

위의 OrderService 와 같은 Application level 의 service 에서 서로 다른 Aggregate 를 직접 의존하는 것도 문제가 될 수 있을까요? 🤔🤔

@@ -1,8 +1,9 @@
package kitchenpos.domain;
package kitchenpos.order.domain;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

용어사전과 모델링을 정의할 때 주문 컨텍스트별로 나누어서 정의했던 것 같은데요,
정의한 내용이 코드상에서도 드러나야 비로소 DDD 가 완성될거 같아요 😄 👍

Comment on lines +1 to 4
package kitchenpos.common.infra;

import kitchenpos.common.infra.PurgomalumClient;
import org.springframework.boot.web.client.RestTemplateBuilder;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비속어가 사용되는 컨텍스트가 정해져있는걸로 기억하는데요,
각각의 컨텍스트에서 중복된 비속어관련 클라이언트를 소지하는 것에 대해서 어떻게 생각하실까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상품과 메뉴 컨텍스트에서 [비속어 클라이언트]를 모두 사용하기 때문에, 클라이언트를 중복 소지하는 관점에 대해서는 미처 생각하지 못 했었네요. 🤔🤔

만약 지금처럼 모놀로틱 아키텍처인 상황에 컨텍스트를 패키지로만 구분하는 상태라면, 최대한 코드 중복을 피하기 위해서 중복으로 소지하지 않는 게 낫지 않을까 싶구요.
멀티 애플리케이션 혹은 MSA 구조라면, 코드 중복을 감안하더라도 중복으로 소지하는 게 나을 수도 있겠다는 생각이 드네요.

혹시 리뷰어님은 어떻게 생각하시나요? 🤔

@LaOnZenaMoon
Copy link
Author

안녕하세요 이준님, 기능 패키지 구성 잘 진행해주셨습니다. 용어사전과 모델링을 정의하면서 고민했던 컨텍스트가 코드에도 반영되면 진짜 DDD 라고 부를 수 있을거 같아요. 관련해서 몇 가지 코멘트 남겼습니다.

코멘트 달아주신 내용을 보고 패키지 구성을 한 것을 다시 보니,
이전 단계에서 고민했던 내용이 반영이 잘 안 되어 있었던 것 같네요 😢

천천히 다시 한 번 고민해서 수정해보겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants