3단계 - 리팩터링(매장 식사 주문) #289

pawoo0211 · 2024-06-01T13:14:43Z

현수님 안녕하세요~ 3단계 진행하여 PR 생성합니다.

먼저 코드가 지저분해져서 보기 어려우실 것 같아 죄송하다는 말씀 먼저 드립니다.. 코드가 지저분해진 이유는 도메인 서비스(ACL)를 강제로 의존하게 되면서 Repository에 주문 테이블이나 메뉴를 반드시 save 해야 하는 강제성 때문에 지저분해졌네요..

부패 방지 레이어(EatInOrderServiceAdapter)를 사용해서 도메인 코드를 보호하려고 했는데 뭔가 보호되지 않은 느낌이라 방향성이 좀 잘못되고 있다고 생각해 현재까지 진행한 부분에 대해서 PR 생성드린 후에 피드백 받아 개선하도록 하겠습니다.

감사합니다.

루트 엔티티를 제외한 하위 엔티티에서는 래핑한 Vo를 반환하도록 변경

hyunssooo

안녕하세요.
추가로 몇 가지 코멘트 남겼습니다.
확인 부탁드려요! 🙂

hyunssooo · 2024-06-06T12:57:13Z

src/main/java/kitchenpos/eatinorders/tobe/domain/entity/OrderLineItem.java

+    @Transient
+    private BigDecimal price;
+
+    @Transient
+    private UUID menuId;


이 경우 메뉴 MC 에서 menu name이 변경되는 경우 어떻게 처리 될까요?

얼마 전에 저희 DM 으로 Menu BC에서의 menu와 Order BC에서 Menu는 의미하는 바가 다름을 도출했는데요. 이를 코드로 녹여보는 것은 어떨까요? Order에 Menu 모델이 만들어 질 수도 있지 않을까요?

hyunssooo · 2024-06-06T12:58:55Z

src/main/java/kitchenpos/eatinorders/tobe/domain/entity/OrderLineItem.java

+    public OrderLineItem(Long seq, long quantity, BigDecimal price) {
+        this(seq, quantity, price, null);
+    }


여러 생성자를 둬 클라이언트에게 편의를 주는 것은 좋은데요. menu는 없어도 괜찮나요?

hyunssooo · 2024-06-06T13:36:40Z

src/main/java/kitchenpos/eatinorders/tobe/domain/repository/OrderTableRepository.java

+
+    Optional<OrderTable> findBy(UUID id);
+}


Suggested change

Optional<OrderTable> findBy(UUID id);

}

Optional<OrderTable> findById(UUID id);

}

hyunssooo · 2024-06-06T14:02:25Z

src/main/java/kitchenpos/eatinorders/tobe/domain/entity/OrderLineItem.java

+    @Transient
+    private BigDecimal price;
+
+    @Transient
+    private UUID menuId;


price와 menuId는 Transient 로 선언되어도 괜찮나요?

hyunssooo · 2024-06-06T14:03:27Z

src/main/java/kitchenpos/eatinorders/tobe/domain/entity/OrderTable.java

+    @Column(name = "occupied", nullable = false)
+    private boolean occupied;
+
+    @Transient
+    private OrderTableStatus status;
+


occupied 와 status 모두 필요할까요?

논리적으로 동일하네요 하하..

hyunssooo · 2024-06-06T14:38:55Z

src/main/java/kitchenpos/eatinorders/tobe/domain/service/OrderTableModification.java

+    @EventListener
+    public void setEmptyTable(CompletedOrderEvent event) {
+        System.out.println("주문 테이블 빈 테이블 설정");


필요하다면 로그를 남기는 것은 어떨까요?

hyunssooo · 2024-06-06T14:44:42Z

src/main/java/kitchenpos/menus/tobe/domain/common/Price.java

+package kitchenpos.menus.tobe.domain.common;



패키지가 조금 어색한 것 같은데요. 모든 BC에서 사용한다면 kitchenpos.common 으로 둬도 괜찮을 것 같긴해요!

제가 좀 설명이 부족했던 것 같은데요. price 클래스가 여러개로 보이네요. BC 별로 정의 한다면 명확하게 정의하는 것이 좋을 것 같고 모든 BC에서 사용하면 kitchenpos.common 에 위치할 수 있을 것 같아요.

hyunssooo · 2024-06-06T15:04:29Z

src/test/java/kitchenpos/eatinorders/domain/EatInOrderCompletePolicyTest.java

+
+@SpringBootTest
+@DisplayName("매장 주문 완료 도메인 서비스 테스트")
+public class EatInOrderCompletePolicyTest {
+    @Autowired
+    private EatInOrderCompletePolicy completePolicy;
+    @MockBean
+    private EatInOrderRepository orderRepository;
+    @MockBean
+    private OrderTableRepository tableRepository;
+
+    private MenuRepository menuRepository;
+    private ToBeFixtures toBeFixtures;
+


꼼꼼한 테스트 👍
EatInOrderCompletePolicy에 ApplicationEventPublisher를 주입 받고 있어 SpringBootTest를 사용하신 것 같네요. 여러방법이 있지만 ApplicationEventPublisher는 인터페이스입니다. 이를 활용해 fake 객체를 만들어 보는 것도 좋을 것 같아요.
혹은 이벤트를 도메인 모델에서 발급해도 괜찮을 것 같아요. AbstractAggregateRoot

별도로 @RecordApplicationEvents , ApplicationEvents 도 찾아보면 좋을 것 같아요.

이 부분은 참고만 부탁드립니다. :)

넵, 말씀 감사합니다!

제가 이벤트 기반이 익숙치 않아서 일단 가장 기본적인 형태로 구현했습니다. 말씀해주신 부분들도 학습해보겠습니다~

hyunssooo · 2024-06-06T15:04:43Z

src/test/java/kitchenpos/eatinorders/domain/FakeEatInOrderRepository.java

+
+public class FakeEatInOrderRepository implements EatInOrderRepository {
+


가짜 객체 👍

hyunssooo · 2024-06-06T15:05:11Z

src/test/java/kitchenpos/eatinorders/domain/OrderLineItemTest.java

+
+@DisplayName("주문 항목 도메인 테스트")
+public class OrderLineItemTest {
+
+    @Test
+    @DisplayName("주문 항목을 생성한다.")
+    void create() {


위에서도 남겼지만 주문 항목에 menu는 필수가 아닌가요?

hyunssooo

안녕하세요.
가벼운 코멘트 몇 가지 남겼습니다!
끝이 보이는데요! 마지막까지 힘내시면 좋겠습니다! 🙂
언제든 DM 남겨주세요!

hyunssooo · 2024-06-08T10:31:53Z

src/main/java/kitchenpos/eatinorders/tobe/domain/vo/MenuInEatInOrders.java

+
+@Embeddable
+public class MenuInEatInOrders {


eatinorders 내부에 Menu 정의 👍

기본 생성자는 필요하지 않을까요?

hyunssooo · 2024-06-08T10:32:09Z

src/main/java/kitchenpos/eatinorders/tobe/domain/vo/MenuInEatInOrders.java

+    @Transient
+    private boolean isDisplayedMenu;
+
+    @Transient
+    private Price price;


Transient 인 이유가 따로 있나요?

주문 테이블에서 위 정보를 굳이 저장할 필요성을 못느껴서 Transient를 적용했습니다.

DM에서도 현수님께서 이미 주문된 메뉴는 변경 가능성이 없다고 하셨는데 저 또한 해당 필드를 영속성으로 유지하거나 DB에 저장할 필요성을 못느껴서 Transient를 적용했습니다.

hyunssooo · 2024-06-08T10:33:42Z

src/main/java/kitchenpos/eatinorders/tobe/domain/service/UpdateMenuInOrders.java

+
+@Component
+public class UpdateMenuInOrders {


이 도메인 서비스는 어떤 경우에 사용되나요?
메뉴의 변경에 따라서 이미 주문된 order 내부의 menu를 모두 update 하는 것 같은데요. 🤔

메뉴의 정보가 바뀌었을 때를 가정하여 만든 도메인 서비스입니다. 실제 요구사항은 없지만 해당 상황을 가정하고 만드는 걸로 이해하여 만들었습니다.

hyunssooo · 2024-06-08T10:41:24Z

src/main/java/kitchenpos/menus/tobe/domain/service/ChangedMenuSender.java

+
+@Component
+public class ChangedMenuSender {
+    private final ApplicationEventPublisher publisher;


강의에서 다뤘을 것 같은데요. AbstractAggregateRoot 통해서 이벤트 발급도 가능합니다. :)
@TransactionalEventListener, @EventListener 키워드도 함께 찾아보면 좋을 것 같아요.
참고만 해주세요~

hyunssooo · 2024-06-08T10:41:58Z

src/test/java/kitchenpos/ToBeFixtures.java


 public class ToBeFixtures {
+    public Menu 메뉴_치킨 = new kitchenpos.menus.tobe.domain.entity.Menu(


Fixtures 도 패키지가 나눠지면 좋을 것 같아요. :)

hyunssooo · 2024-06-08T16:06:09Z

src/main/java/kitchenpos/eatinorders/tobe/domain/service/EatInOrderAccept.java

+
+public class EatInOrderAccept {
+    private OrderTableRepository tableRepository;


이 클래스는 EatInOrder를 생성하는 시점에 사용될 것 같아요.

MenuInEatInOrders의 생성 시점은 언제인가요? 🤔

bean으로 등록되지 않아도 괜찮나요?

MenuInEatInOrders 생성 시점도 EatInOrders와 동일한 생성 시점으로 생각하고 있습니다.

MenuInEatInOrders에 대한 유효성을 검증하기 위해서는 EatInOrder 객체가 생성되어 하기에 EatInOrderTest 클래스에서 유효성 검증 테스트 코드 작성했습니다.

hyunssooo · 2024-06-08T16:09:43Z

src/main/java/kitchenpos/menus/tobe/domain/common/Price.java

+package kitchenpos.menus.tobe.domain.common;



제가 좀 설명이 부족했던 것 같은데요. price 클래스가 여러개로 보이네요. BC 별로 정의 한다면 명확하게 정의하는 것이 좋을 것 같고 모든 BC에서 사용하면 kitchenpos.common 에 위치할 수 있을 것 같아요.

hyunssooo · 2024-06-08T16:18:41Z

src/test/java/kitchenpos/eatinorders/domain/OrderTest.java

+@DisplayName("주문 도메인 테스트")
+public class OrderTest {
+


테스트 클래스 이름이 EatInOrderTest 가 아니네요. 🤔

아래에 대한 요구 사항은 어떤 클래스의 책임일까요? 테스트로 작성되어야 하지 않을까요?
주문한 메뉴의 가격은 실제 메뉴 가격과 일치해야 한다.

hyunssooo · 2024-06-08T16:21:47Z

src/main/java/kitchenpos/eatinorders/tobe/domain/entity/OrderLineItem.java

+    @Column(name = "quantity", nullable = false)
+    private long quantity;
+
+    @Embedded
+    private Price price;
+
+    @Embedded
+    private MenuInEatInOrders menuInOrders;


궁금해서 그러는데 price는 MenuInEatInOrders.price와 동일한가요? 그렇다면 중복인 것 같은데요.
quantity * MenuInEatInOrders.price 라면 직접 계산할 수 도 있을 것 같아서요.

pawoo0211 added 30 commits May 23, 2024 22:03

test: 주 생성자 사용 로직으로 수정

b7c9083

test: 테스트 코드를 위한 메서드 제거

c43e274

test: 이름, 가격 도메인 테스트 코드 작성

f2260de

refactor: To Be 메뉴 그룹 구현

ab73347

refactor: To Be 메뉴 상품 구현

623bad4

refactor: To Be 메뉴 구현

6fd402e

refactor: DisplayedName -> CleanName으로 네이밍 변경

abd44e0

refactor: DisplayedName 제거

eafef3d

refactor: 메뉴 숨김 getter 로직 제거

a95b239

test: 랜덤 객체 재사용으로 수정

7a34cd0

test: 상품 생성 메서드 제거

b0e7f58

test: 메뉴 도메인 이름 테스트 코드 추가

03009d2

refactor: 메뉴 -> 메뉴 그룹 id 의존으로 변경

993630b

refactor: 메뉴 상품 -> 상품 id 의존 변경에 따른 1차 리팩터링

fdf5132

refactor: 메뉴 상품 -> 상품 id 의존 변경에 따른 2차 리팩터링

940bbc0

refactor: 메뉴 상품 -> 상품 객체 의존 제거

6a3bc8c

chore: 메뉴 상품 리팩터링 테스트 코드 정리

34f17f8

refactor: 메뉴 상품 가격 반환 리팩터링

f718f35

루트 엔티티를 제외한 하위 엔티티에서는 래핑한 Vo를 반환하도록 변경

refactor: 메뉴 상품 가격과 메뉴 가격 비교 메서드 위치 및 네이밍 변경

b7d6076

test: 가격 도메인 추가 테스트 코드 작성

bb80df0

refactor: 두번째 미션 마지막 피드백 적용

af38e43

feat: To Be 주문 테이블 구현

443acc1

feat: To Be 주문 항목 뼈대 코드 작성

5cd6899

feat: To Be 주문 뼈대 코드 작성

6f1d289

refactor: To Be 패키지 구조 변경

ce1c3af

refactor: 주문 등록 전 빈 테이블 확인 및 주문 완료 후 빈 테이블 설정 구현

6d21ef3

refactor: 주문 ACL 클래스에 따른 패키지 구조 변경

5107e75

refactor: 주문 ACL 클래스에 따른 메뉴 패키지 구조 변경

49830eb

feat: 주문 생성 시 메뉴 검증 구현

9b09de2

test: 테스트 전 저장해야 하는 데이터 셋 코드 정리

3f16f60

pawoo0211 added 23 commits June 6, 2024 08:46

test: 생성자를 이용한 주문 상태 변경

eccbe2b

test: 테스트 코드를 위한 메서드 제거

07ff076

test: 이름, 가격 도메인 테스트 코드 작성

a80da1b

refactor: To Be 메뉴 그룹 구현

9962450

refactor: To Be 메뉴 상품 구현

b796d70

refactor: To Be 메뉴 구현

21b8f13

refactor: DisplayedName -> CleanName으로 네이밍 변경

babffa7

refactor: DisplayedName 제거

6986496

refactor: 메뉴 숨김 getter 로직 제거

5e1603f

test: 랜덤 객체 재사용으로 수정

f256d6a

test: 상품 생성 메서드 제거

715b933

test: 메뉴 도메인 이름 테스트 코드 추가

cd012ad

refactor: 메뉴 -> 메뉴 그룹 id 의존으로 변경

fc1c97a

refactor: 메뉴 상품 -> 상품 id 의존 변경에 따른 1차 리팩터링

8568977

refactor: 메뉴 상품 -> 상품 id 의존 변경에 따른 2차 리팩터링

fce2e2e

refactor: 메뉴 상품 -> 상품 객체 의존 제거

cf50ce9

chore: 메뉴 상품 리팩터링 테스트 코드 정리

eeb4fce

refactor: 메뉴 상품 가격 반환 리팩터링

a4257d5

루트 엔티티를 제외한 하위 엔티티에서는 래핑한 Vo를 반환하도록 변경

refactor: 메뉴 상품 가격과 메뉴 가격 비교 메서드 위치 및 네이밍 변경

fe08dd9

test: 가격 도메인 추가 테스트 코드 작성

ea91181

Merge remote-tracking branch 'origin/step3' into step3

e732a74

fix: 머지 충돌 해결

5e84dc8

fix: 누락된 커밋 추가

3286a41

hyunssooo requested changes Jun 6, 2024

View reviewed changes

pawoo0211 added 5 commits June 8, 2024 00:04

fix: 주문 항목 메뉴 식별자 및 금액 예외 처리

bd53968

fix: 주문 항목 금액과 메뉴 금액 일치 여부에ㄷ 따른 수정

9cd603f

refactor: 메뉴 변경에 따른 주문 검증

2a3a17c

refactor: 동일한 사용에 따라 OrderTableStatus 제거

19c576d

chore: 이벤트 핸들링 시 로깅 처리

22d0891

hyunssooo requested changes Jun 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3단계 - 리팩터링(매장 식사 주문) #289

3단계 - 리팩터링(매장 식사 주문) #289

pawoo0211 commented Jun 1, 2024

hyunssooo left a comment

hyunssooo Jun 6, 2024

hyunssooo Jun 6, 2024

hyunssooo Jun 6, 2024

hyunssooo Jun 6, 2024

hyunssooo Jun 6, 2024

pawoo0211 Jun 7, 2024

hyunssooo Jun 6, 2024

hyunssooo Jun 6, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 6, 2024

pawoo0211 Jun 7, 2024

hyunssooo Jun 6, 2024

hyunssooo Jun 6, 2024

hyunssooo left a comment

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024

pawoo0211 Jun 9, 2024

hyunssooo Jun 8, 2024

pawoo0211 Jun 9, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024

pawoo0211 Jun 9, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024

hyunssooo Jun 8, 2024


		public class FakeEatInOrderRepository implements EatInOrderRepository {


		public class ToBeFixtures {
		public Menu 메뉴_치킨 = new kitchenpos.menus.tobe.domain.entity.Menu(


		public class EatInOrderAccept {
		private OrderTableRepository tableRepository;

		@DisplayName("주문 도메인 테스트")
		public class OrderTest {

		package kitchenpos.menus.tobe.domain.common;

		package kitchenpos.menus.tobe.domain.common;

3단계 - 리팩터링(매장 식사 주문) #289

Are you sure you want to change the base?

3단계 - 리팩터링(매장 식사 주문) #289

Conversation

pawoo0211 commented Jun 1, 2024

hyunssooo 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyunssooo 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment