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

hoa0217 · 2024-06-21T20:37:28Z

안녕하세요 리뷰어님.
매장 식사 주문 리팩토링 완료하여 PR 올립니다.
tobe 패키지를 만들어서 OrderTable, StoreOrder를 수정한 후 application 계층까지 수정완료하였습니다.

common 패키지에 Order라는 abstract class를 만들어서 이를 상속받아 각 주문, 배달, 포장 Order 엔티티를 구현하는 방식으로 구현해보려하였습니다. 또한 Menu리팩토링의 필요성이 느껴져 아래와 같이 개선했습니다.

MenuProduct에 Price 프로퍼티를 추가하여 총합계산의 책임을 MenuProduct가 가지게 하였습니다.
MenuProductSerivce에 나와있는 비지니스로직을 MenuProduct로 옮긴 후 제거하였습니다.

감사합니다!

- JPA Entity에는 붙일 수 없다.

- MenuProducts는 MenuPrice를 몰라도 된다. 의존성 제거.

…뉴를 숨긴다.

- VO(NumberOfGuest, OrderTableName)을 갖는다.

- 해당 수량은 메뉴에 대한 수량이 아닌, 메뉴상품안에 상품에 대한 수량이다.

- 주문클래스를 상속받으며 매장주문용 비지니스로직을 갖는다.

- 매장주문을 생성한다

- dto에 있는 불변식 검증을 도메인으로 옮긴다.

liquidjoo

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

liquidjoo · 2024-06-24T00:10:43Z

src/main/java/kitchenpos/menus/domain/tobe/MenuProducts.java

+    private void validateMenuProducts(List<MenuProduct> menuProducts, List<Product> products) {
+        if (menuProducts == null || products == null) {
            throw new IllegalArgumentException();
        }
+
+        if (products.size() != menuProducts.size()) {
+            throw new IllegalArgumentException();
+        }
+
+        if (menuProducts.isEmpty()) {
+            throw new IllegalArgumentException();
+        }
+    }


둘 이상의 도메인이 엮인다면 도메인 서비스를 활용해 보아도 좋을 것 같네요 :)

도메인 서비스 도출해보겠습니다!

liquidjoo · 2024-06-24T00:11:19Z

src/main/java/kitchenpos/menus/domain/tobe/MenuProducts.java

+    public Stream<MenuProduct> stream() {
+        return menuProducts.stream();


Stream 으로 반환한 이유가 있을까요?
Stream 으로 반환 시 어떤 장단점이 있을까요?

장점으로 Stream으로 반환 시 getter를 쓰지않고 일급컬렉션을 탐색할 수 있습니다~
단점으로는.. for문을 돌리는 것보다 성능이 안좋으려나요 ㅎㅎ 또한 디버깅이 어려울 수 있겠네요!
일단 Iterable을 구현했으니 해당 기능을 사용하도록 수정하겠습니다!

혹 제가 모르는 부분이 있다면 조언부탁드립니다!

liquidjoo · 2024-06-24T00:14:33Z

src/main/java/kitchenpos/orders/common/application/dto/OrderLineItemRequests.java

+    public List<UUID> getMenuIds() {
+        return orderLineItems.stream().map(OrderLineItemRequest::getMenuId).toList();
+    }
+
+    public List<OrderLineItem> toOrderLineItems(List<Menu> menus, OrderType orderType) {
+        return orderLineItems.stream()
+                .map(orderLineItem -> orderLineItem.toStore(findMenu(menus, orderLineItem),
+                        orderType)).toList();
+    }


개인적으론 Dto 에선 비즈니스 로직을 최대한 배제하는 편인데요 :)
저의 생각은 Dto 는 객체라기 보단 데이터 전달을 해주는 구조체에 가깝다고 생각하고 있습니다
정화님은 어떻게 생각하시나요?

맞습니다. DTO는 데이터 전달용 객체이지 비지니스로직은 도메인계층에서 가져야합니다. 비지니스로직은 도메인으로 엔티티 생성 책임은 어플리케이션으로 옮기겠습니다!

liquidjoo · 2024-06-24T00:15:37Z

src/main/java/kitchenpos/orders/common/application/dto/OrderLineItemRequests.java

+    private Menu findMenu(List<Menu> menus, OrderLineItemRequest orderLineItem) {
+        return menus.stream().filter(menu -> menu.getId().equals(orderLineItem.getMenuId()))
+                .findFirst().orElseThrow((NoSuchElementException::new));
+    }


Dto 에서 다른 영역의 도메인을 의존하고 있는 것 같네요
Order 라는 영역에서 Menu 에 대한 도메인을 가지고 있다면 어떤 단점이 있을까요?

일단 DTO에서 엔티티 생성에 대한 책임은 제거하도록하겠습니다!
그러나 OrderLineItem이 menuId에 대한 참조를 가지고 있다보니.. 올바른 Menu인지 아닌지 검증은 필요할거같은데 팩토리 클래스를 도출하여 해당 작업을 진행하는게 좋으려나요ㅠㅠ

OrderLineItemMapper클래스(어플리케이션 서비스)를 도출해서 해당 작업을 진행하였습니다!

liquidjoo · 2024-06-24T00:18:23Z

src/main/java/kitchenpos/orders/common/domain/tobe/Order.java

+    @Column(name = "type", nullable = false, columnDefinition = "varchar(255)")
+    @Enumerated(EnumType.STRING)
+    private OrderType type;
+
+    @Column(name = "status", nullable = false, columnDefinition = "varchar(255)")
+    @Enumerated(EnumType.STRING)
+    private OrderStatus status;


모델링을 보았을 때도 주문 유형에 따라 필요한 주문 상태 값이 다르다는 걸 인지하셨는데요~
그럼 OrderType 과 OrderStatus 는 공통으로 볼 수 있을까요?

두개를 분리할까 고민을 했지만, Order라는 추상클래스에서 공통으로 쓰이는 속성이 맞다고 판단을 했습니다.

OrderType: OrderType의 경우 Order를 상속하는 클래스 타입을 구분하는 컬럼으로 사용되고있어 제가 상속을 선택한 이상 common에 있을 필요가 있다고 생각했습니다.

OrderStatus: 주문 종류별로 다른 EnumType을 쓰게되면 상속 시 싱글테이블전략을 못쓰고 테이블을 분리해야하더라구요.. 또한 중복되는 상태들이 다수로 존재하는데 이를 따로 클래스로 빼는게 맞는걸까?라는 고민이 들었습니다.

liquidjoo · 2024-06-24T00:19:01Z

src/main/java/kitchenpos/orders/store/domain/tobe/StoreOrder.java

+import kitchenpos.orders.common.domain.tobe.OrderLineItem;
+
+@Entity
+public class StoreOrder extends Order {


추상 클래스의 활용 👍
다른 주문 타입들도 추가해보면 어떨까요?

넵 추가 후 리뷰 요청하겠습니다!! 감사합니다

- dto의 엔티티 생성로직 제거

…Validator 클래스명 변경

hoa0217 added 30 commits June 19, 2024 02:09

refactor(MenuProduct): MenuProduct는 가격을 갖는다.

ecf8396

refactor(Menu): 메뉴는 스스로 메뉴가격을 검증할 수 있다

1667721

fix(MenuProduct): 필드 final 제거

ff0d84b

- JPA Entity에는 붙일 수 없다.

fix(MenuProducts): MenuPrice 의존성 제거

c3f2e8b

- MenuProducts는 MenuPrice를 몰라도 된다. 의존성 제거.

fix(Menu): MenuPrice 검증은 Menu가 한다.

db93f01

fix(Menu): MenuPrice 변경 시 변경된 가격을 검증한다.

dc1d39b

fix(Menu): MenuProduct의 ProductPrice를 변경한다. 이때 메뉴가격이 Product총합보다 크면 메…

3220bc1

…뉴를 숨긴다.

refactor(OrderTableName): 주문테이블의 이름은 없거나 비어있을 수 없다.

324233f

refactor(NumberOfGuests): 주문테이블의 손님은 0보다 작을 수 없다.

ebe2ef2

refactor(OrderTable): 주문테이블 클래스를 새로 생성한다.

59771a5

- VO(NumberOfGuest, OrderTableName)을 갖는다.

chore(OrderTable): 패키지명 변경 eatin -> store

4a51c8d

refactor(Order): 공통으로 사용되는 Order클래스 생성

60d3e0f

fix(ProductQuantity): MenuQuantity -> ProductQuantity로 변경

dc494e0

- 해당 수량은 메뉴에 대한 수량이 아닌, 메뉴상품안에 상품에 대한 수량이다.

fix(OrderLineItem): 노출되어있지 않은 메뉴로 주문라인아이템을 생성할 수 없다.

e6b31cf

test(MenuQuantity): 매장주문이 아닌 주문의 메뉴수량은 음수가 될 수 없다.

92c866d

feat(StoreOrder): 매장주문용 클래스 생성

72878da

- 주문클래스를 상속받으며 매장주문용 비지니스로직을 갖는다.

feat(StoreOrder): 매장주문용 레파지토리 생성

1c9cbc7

feat(OrderTable): 주문테이블 서비스 리팩토링

4825a7f

feat(StoreOrder): 매장주문 전용 서비스 클래스 생성

6d5dce0

- 매장주문을 생성한다

fix(OrderTable): 테이블이 점유되어있지 않은 상태에서 손님수를 변경하면 예외를 던진다

80a6f64

refactor(OrderTable): 패키지 경로 tobe로 변경 및 포맷맞추기

a9e0438

test(OrderTable): 점유되지않은 테이블의 손님수를 변경하면 예외를 던진다

30223f0

test(StoreOrder): 매장주문을 생성할 수 있다

3127efa

feat(StoreOrder): 매장주문을 접수, 전달, 완료를 할 수 있다.

378409e

fix(StoreOrder): 테스트케이스 용어 변경

3cd532c

fix(Menu): 사용하지 않는 생성자 제거

8a4d5b2

fix(Menu): dto 패키지 ui -> application으로 이동

108f673

fix(Product): dto 패키지 ui -> application으로 이동

ddcc764

fix(StoreOrder): 매장주문 생성시 메뉴가 겹치면 예외를 던진다.

b0633a8

- dto에 있는 불변식 검증을 도메인으로 옮긴다.

fix(MenuProduct): 메뉴상품 생성시 상품이 겹치면 예외를 던진다.

40dcdc0

- dto에 있는 불변식 검증을 도메인으로 옮긴다.

hoa0217 added 3 commits June 23, 2024 03:17

fix(MenuProduct): 메뉴상품 서비스 제거

7845997

refactor(MenuProduct): 안쓰는 메서드 정리

74024f7

feat(StoreOrder): Controller 생성

b93eb1a

liquidjoo requested changes Jun 24, 2024

View reviewed changes

hoa0217 added 9 commits July 2, 2024 15:19

feat(MenuProductValidator): 메뉴상품을 검증하는 도메인 서비스 생성

9873af7

refactor(MenuProductMapper): 요청을 받아 메뉴 상품을 생성하는 어플리케이션 서비스 생성

338db62

- dto의 엔티티 생성로직 제거

refactor(MenuProducts): stream메서드 제거

9074bc6

refactor(MenuProductsValidator): MenuProductValidator -> MenuProducts…

ebd1a62

…Validator 클래스명 변경

refactor(MenuCreateRequest): Validation 어노테이션 추가

04a0359

feat(OrderLineItemsValidator): 주문 아이템을 검증하는 도메인 서비스 생성

e2ab9c1

refactor(OrderLineItemMapper): 요청을 받아 주문 아이템을 생성하는 어플리케이션 서비스 생성

047fd52

refactor(StoreOrderRestController): Validation 어노테이션 추가

d0c8174

test(OrderLineItemsValidator): 주문 아이템을 검증하는 도메인 서비스 생성

247cbc0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hoa0217 commented Jun 21, 2024

liquidjoo left a comment

liquidjoo Jun 24, 2024

hoa0217 Jul 1, 2024

liquidjoo Jun 24, 2024

hoa0217 Jul 1, 2024

liquidjoo Jun 24, 2024

hoa0217 Jul 1, 2024 •

edited

Loading

liquidjoo Jun 24, 2024

hoa0217 Jul 2, 2024 •

edited

Loading

liquidjoo Jun 24, 2024

hoa0217 Jul 2, 2024 •

edited

Loading

liquidjoo Jun 24, 2024

hoa0217 Jul 2, 2024

		public Stream<MenuProduct> stream() {
		return menuProducts.stream();

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

Are you sure you want to change the base?

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

Conversation

hoa0217 commented Jun 21, 2024

liquidjoo 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

hoa0217 Jul 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoa0217 Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoa0217 Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoa0217 Jul 1, 2024 •

edited

Loading

hoa0217 Jul 2, 2024 •

edited

Loading

hoa0217 Jul 2, 2024 •

edited

Loading