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단계 - 기능 우선 패키지 #411

Open
wants to merge 2 commits into
base: wooddy-kim
Choose a base branch
from

Conversation

wooddy-kim
Copy link

작업 내역

  • BOUNDED CONTEXT 중심의 패키지 구조로 변경

궁금한 점

  1. 주문 도메인을 분리하면서 일부 동작하지 않는 코드가 생겼습니다. 이 부분을 수정해야 할까요? 이번 미션에서는 크게 중요하지 않은 것 같아서 무시 했습니다.
  2. 주문 도메인을 분리할 때 중복 코드를 제거하기 위해 처음에는 Order 클래스를 두고 상속을 사용했었습니다. 그러나 수업 시간에 '우연히 겹친 것'이라는 말씀을 듣고 모델링과 동일하게 별도 클래스로 분리했습니다. 이 중에서 OrderLineItem과 같은 중복되는 부분은 common과 같은 패키지로 분리하여 재사용하는 것이 좋을까요?

Copy link

@liquidjoo liquidjoo left a comment

Choose a reason for hiding this comment

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

안녕하세요 재우님 :)
3단계 미션 잘해주셨어요~
몇 가지 코멘트 남겼습니다
확인하고 다시 요청 주세요 🙇

Comment on lines +8 to +9
import kitchenpos.domain.product.Product;
import kitchenpos.domain.product.ProductRepository;

Choose a reason for hiding this comment

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

menu service 영역에 product 패키지가 의존되는 것 같은데요
이런 부분도 한번 의존성을 제거해보면 어떨까요?

Comment on lines +3 to +4
import kitchenpos.domain.menu.Menu;
import kitchenpos.domain.menu.MenuRepository;

Choose a reason for hiding this comment

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

주문서 서비스에 menu 패키지가 의존되는 것을 어떻게 막아볼 수 있을까요?

Comment on lines +3 to +5
import kitchenpos.domain.menu.Menu;
import kitchenpos.domain.menu.MenuProduct;
import kitchenpos.domain.menu.MenuRepository;

Choose a reason for hiding this comment

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

위와 동일합니다!
menu 의 의존을 제거해보아요~

@@ -10,6 +10,7 @@
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
import jakarta.persistence.Transient;
import kitchenpos.domain.product.Product;

Choose a reason for hiding this comment

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

MenuProduct 와 Product 는 같은 영역의 컨텍스트일까요?

import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
import jakarta.persistence.Transient;
import kitchenpos.domain.menu.Menu;

Choose a reason for hiding this comment

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

다음 미션인 리팩터링의 영역을 조금 침범할 수 도 있지만 고려해보면 좋을 것 같아요 :)


@Table(name = "order_line_item")
@Entity
public class DeliveryOrderLineItem {

Choose a reason for hiding this comment

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

새롭게 만들어진 영역의 컨텍스트가 있으면 테스트 코드 또한 작성해봐야 좋을 것 같습니다

테스트 코드가 리팩터링 후에도 유효해야하기 때문에 작성하는 것을 추천합니다!

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