-
Notifications
You must be signed in to change notification settings - Fork 196
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단계 - 기능 우선 패키지 구성하기 #484
base: restinbeat
Are you sure you want to change the base?
3단계 - 기능 우선 패키지 구성하기 #484
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생 많으셨습니다 💯
몇가지 코멘트 남겼습니다. 한번 확인 부탁드려요~
@@ -1,4 +1,4 @@ | |||
package kitchenpos.infra; | |||
package kitchenpos.global.infrastructure.external; | |||
|
|||
public interface PurgomalumClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메뉴서비스가 인프라 레이어에 직접 의존하고 있네요.
DIP에 대해서 고민해보셨으면 좋을 것 같아요.
@@ -1,4 +1,4 @@ | |||
package kitchenpos.domain; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
menu와 product의 바운디드 컨택스트를 분리해보는것은 어떨까요?
import kitchenpos.order.domain.entity.OrderStatus; | ||
import kitchenpos.order.domain.entity.OrderTable; | ||
import kitchenpos.order.domain.entity.OrderType; | ||
import kitchenpos.menu.domain.entity.Product; | ||
|
||
public class Fixtures { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
비지니스의 확장을 고려해서 fixture도 분리하는게 어떨까요?
@@ -1,6 +1,8 @@ | |||
package kitchenpos.domain; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분도 주문 도메인이 인프라에 의존하고 있는데요. 어떻게 생각하세요?
@@ -1,4 +1,4 @@ | |||
package kitchenpos.domain; | |||
package kitchenpos.order.domain.entity; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
키친 포스의 요구사항은 매우 간단하기 때문에 문제가 안되지만, 미션인 만큼 앞으로 비지니스의 확장을 염두해보면 좋을 것 같은데요.
주문의 속성은 같을 수 있지만 비지니스의 확장이 갈수록 매우 달라질 것 같아요.
그렇다면 주문 타입에 따라 바운디드 컨택스트를 나눠보는것은 어떨까요?
import kitchenpos.menu.domain.repository.MenuRepository; | ||
import kitchenpos.menu.domain.entity.Product; | ||
import kitchenpos.menu.domain.repository.ProductRepository; | ||
import kitchenpos.global.infrastructure.external.PurgomalumClient; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
흐름을 제어하는것은 application layer 의 역할이 아닐까요?
도메인 레이어의 서비스로 넣으신 이유가 있을까요?
리뷰어님, 안녕하세요!
3단계 - 기능 우선 패키지 구성하기 PR 올립니다.
step3 - 미션 수행 기준
BOUNDED CONTEXT 중심으로 패키지 분리하였고,
추가적으로 퍼사드패턴 적용해서 */application/facade에 서비스들 결합해서 유스케이스로 다음 미션 진행하려고 합니다.
기타 의견이나 참고사항 있으시면 피드백 부탁드려요 ! 감사합니다. 😊