[레거시 코드 리팩터링 - 2단계] 제이미(임정수) 미션 제출합니다. #657

JJ503 · 2023-10-25T07:05:32Z

안녕하세요 오리!
최종 데모데이가 마무리된 후 시작을 했는데 벌써 수요일이 되어 버렸네요... 😨
너무 늦게 요청드려 오리의 일정에 방해가 되지 않길 바랄 뿐입니다...!

이번 2단계에서 수정한 내용은 다음과 같습니다.

setter 메서드 제거 및 네이밍 수정
JPA 레파지토리로 변경
- 이에 따른 서비스 테스트 수정 (단일 -> 통합)
  -> 해당 경우 dao를 사용한 레파지토리 관련 테스트는 단일 테스트를 수행해야 한다고 생각했지만, JPA를 사용하게 된다면 도메인에도 연관관계가 있기에 통합 테스트를 수행해야 한다고 생각해 수정했습니다. (확실히, 다른 분들이 걱정한 대로 테스트 수정에 있어 힘들긴 했네요.. 🥲)
컨트롤러에 요청/응답 dto 추가 및 수정
서비스 코드 리팩터링 (도메인으로 로직 분리)

그런데 서비스 코드 리팩터링의 경우 가장 마지막에 수행하다 보니 마음이 급해져 정신없이 한 바람에 제대로 못한 것 같기도 하네요... 🥲

수행하면서 특히 의문이 들었던 점은 검증 로직이었습니다.
전달된 dto의 값을 통해 검증할 수 있는 부분에 대해서 오리는 어떻게 진행하셨나요?
dto에서 검증을 해결할 수 있다면 find~()와 같이 비용이 드는 작업 전에 하는 게 더 좋지 않을까?라는 의문이 들어 여쭤봅니다.
하지만, 일단 이번 미션에서는 각 도메인에서 해줘야 할 것으로 생각하는 작업들에 대해선 도메인으로 로직을 분리하려 노력해 보았습니다.
사실 request dto에 @valid를 사용해 진행한다면 좀 해결이 될 것 같기도 하지만, 일단 시간이 없기도 해서 생략했습니다.

추가적으로 fixture를 어떻게 하면 의미 있게 사용할 수 있을까에 대해 계속 고민해 봤는데 생각보다 어렵더군요...
일단 지난번에 말씀해 주신 대로 중복되는 문제를 해결하는 데 초점을 두어 진행해 보았습니다!

이번 미션도 리뷰 잘 부탁드립니다!

…리팩터링

carsago

안녕하세요~ 제이미 2단계 요구사항 작성하시느라 고생하셨습니다.
리뷰 요청을 늦게하시나 일찍 하시나 하는건 똑같기 때문에 전 별상관없습니다 ㅋㅋ;
현재 코드에서 마음에 안드시는 부분도 많으실텐데요. 전 3단계를 수행하면서 자연스럽게 개선 될 부분이 많다고 느껴져요.

다만 간단한 코멘트 달았으니, 이번 단계에선 몇몇 부분만 고려해주실 바랍니다 !

carsago · 2023-10-25T09:10:02Z

src/main/java/kitchenpos/domain/Menu.java

 public Menu(
 final String name,
- final BigDecimal price,
- final Long menuGroupId,
+ final Price price,
+ final MenuGroup menuGroup
+ ) {
+ this.name = name;
+ this.price = price;
+ this.menuGroup = menuGroup;
+ }


이 생성자는 테스트를 위해 존재하는 코드지 않을까 생각이드네요. 주의하면 되겠지만, 그런 부분이 하나, 하나씩 생길 때 마다 이해해야 하는 컨텍스트가 많아질테니까요..

맞습니다…
사실 마지막에 이를 인지하긴 했는데 당시에 너무 힘들어서 그냥 흐린눈하고 넘어가버렸네요 ㅎㅎ

수정 완료했습니다!

carsago · 2023-10-25T09:15:20Z

src/main/java/kitchenpos/domain/Menu.java

+ public static Menu of(
+ final String name,
+ final Price price,
+ final MenuGroup menuGroup,
+ final List<MenuProduct> menuProducts
+ ) {
+ final Menu menu = new Menu(name, price, menuGroup, menuProducts);

- public void setId(final Long id) {
- this.id = id;
+ validateMenuProduct(menuProducts, price);
+ addMenuProducts(menuProducts, menu);
+
+ return menu;
 }


저는 객체 생성을 static factory method를 통하는 것이 어떤 메리트가 있을까? 라는 생각이 듭니다.
물론 OrderStatus에 특정한 값을 부여한다는 식으로, (생성자로는 불가능한) 메소드 명을 통한 의미 부여가 가능하다면 충분히 합리적이지만, 지금은 단순하게 객체를 생성할 뿐이잖아요?

그리고 우리가 코드를 볼 때, 객체를 생성하는 방식은 일반적으로 누구나 그렇듯, 생성자를 떠올릴 것이기 때문에 static factory method를 통해 생성해야한다면, 이해해야하는 포인트가 많아진다고 여겨져요.

또 현재는 생성자가 아닌 static factory method에 의해서만 validate를 하고 있기 때문에, 누군가가 생성자를 통해 객체를 만들어낸다면, 비즈니스 룰상 객체가 생성되지 않아야 함에도 생성되는 경우가 생길 수도 있을 거 같아요.

개인적으로 검증로직이 들어가게 된다면 정적팩토리메서드로 빼는 것이 익숙해진 상태입니다.
그리고 이를 사용하게 된 의미를 생각해 보면 단순히 객체를 생성할 뿐만 아니라 검증을 진행하고 있기 때문입니다.
저라면 생성자에게 기대하는 것은 정말 객체를 생성하는 것이라고 생각합니다.
그래서 생성 시 검증으로 인해 문제가 생긴다면 이에 대한 의문을 품게 되지 않을까 생각했습니다.
또한, 추후 검증 없이 객체를 생성해야 할 수도 있습니다.
현재는 정적팩토리메서드가 한 개이기에 of라는 기본 네이밍을 사용했지만, 검증이 필요하지 않은 생성 방식이 필요하다면 생성자를 새로 만들거나 그때가 돼서야 정적팩토리메서드를 생성하는 것보다, 네이밍으로 구분한 정적팩토리메서드가 두 개가 생기는 것이 더 효율적이라고 생각합니다.

하지만, 현재 생성자가 public으로 되어 있어 사용자가 비즈니스 룰상 생성되지 말아야 할 때도 생성되는 것이 문제라는 의견에 대해선 동의합니다.
해당 부분도… 당시에 수정하기 너무 힘들어서 픽스처 때문에 열어둔 상태라 다시 private으로 생성자를 통해 객체를 생성하는 것에 대해선 막아두었습니다.

carsago · 2023-10-25T09:21:37Z

src/main/java/kitchenpos/application/MenuService.java

+ final List<Menu> menus = menuRepository.findAll();

 for (final Menu menu : menus) {
- menu.setMenuProducts(menuProductDao.findAllByMenuId(menu.getId()));
+ menu.updateMenuProducts(menuProductRepository.findAllByMenuId(menu.getId()));
 }



이거 fetch join으로 한번에 처리하는건 어떨까요?
사실 join쿼리 안날리고 미션코드라 성능 상관없으니 response dto mapping 할때 n+1 문제 나도 별 상관없다는 생각은 드네요..

맞습니다!
사실 JPA로 변경되게 되면서 고려해야 할 부분들이 몇 가지 있었지만 일단 이번 단계에서는 크게 신경 쓰지 않았습니다.
해당 부분은 3단계를 진행하며 시간적 여유가 된다면 고려할 수 있도록 하겠습니다..!

carsago · 2023-10-25T10:38:29Z

src/test/java/kitchenpos/application/OrderServiceTest.java

+ final Order order = orderRepository.save(OrderFixture.조리_상태의_주문_엔티티_생성(orderTable, menu));
+ final ChangeOrderStatusRequest changeStatusRequest = new ChangeOrderStatusRequest(OrderStatus.MEAL.name());


이건 취향일 수 있는데, 저 같은 경우 이렇게 클래스명이 길다면 가독성을 위해 var를 사용하는 편이에요.

타입을 명시하지 않기 때문에 코드를 이해하기 어려워 지는 경우도 있지만, 이런 경우 우항에서 바로 생성자를 호출해주기 때문에, 그런 부분은 없으니까요.

다만 이 부분은 스타일이니까 제이미의 취향에 맞게 해주세요!!

저는 개인적으로 var를 사용하는 것은 좋지만, 타입을 명시해 주는 것과 var를 작성하는 명확한 기준이 없다면 혼용하는 것을 선호하지 않습니다.
왜냐하면, 기준이 없을 시 오히려 코드를 읽는 사람들로 하여금 왜 여기만 타입을 명시하지 않은 것인데 한 번 더 생각하게 될 것이고, 작성자 입장에서는 var를 사용해야 하는 경우인지 아닌 경우인지 계속해 고민을 해야 한다는 문제가 생길 것이라 생각하기 때문입니다.
그래서 현재로서 var를 사용할 것이라면 전체적으로 사용해야 한다고 생각해 이는 모든 코드를 수정해야 하기에 너무 많은 비용이 발생한다고 생각합니다.

그렇다면 오리의 경우 var를 사용한다의 기준은 무엇인가요?
클래스명이 길다라는 것은 너무 추상적이고 코드를 작성하는 사람에 따라 기준이 휙휙 변할 수 있다고 생각해 정확히 어떤 기준이 있는 것인지 궁금합니다!
또한, 만약 협업 시 가독성을 위해 해당 컨벤션을 만드셨다면 이가 잘 진행되었는지도 궁금합니다!

https://openjdk.org/projects/amber/guides/lvti-style-guide

저는 말씀드린것 처럼 같은 라인에서 생성자를 호출할 때 선호하는 편이에요.
가독성이 이라는게 주관적이라지만, 역설적으로 동시에 보편적인 기준이기도 해요. 디자인등을 봐도 그렇죠.

오 좋은 자료 감사합니다!

긴 경우라고 해서 생성자 호출 시 긴 경우에만 적용된다라는 의미로 잘못 받아들였네요..!
이번에는 시간상 var 적용이 어려울 것 같지만, 오리 덕분에 var 타입에 대해서도 좀 더 생각해 볼 수 있었습니다.

carsago · 2023-10-25T10:53:02Z

src/main/java/kitchenpos/application/OrderService.java

+ orderTableRepository.findById(orderRequest.getOrderTableId())
+ .orElseThrow(() -> new NotFoundOrderTableException("해당 주문 테이블이 존재하지 않습니다."));
+ final List<OrderLineItem> orderLineItems = convertToOrderLineItem(orderRequest);
+ final Order order = orderRequest.toEntity(orderTable, OrderStatus.COOKING, LocalDateTime.now(), orderLineItems);


저는 전에는 DTO에 toEntity라는 메소드를 넣는게 큰 상관이 없다고 여겼는데요. (코드가 간소화되니까)
이런 상황에선 더욱더 해당 객체의 생성자에 하나, 하나씩 DTO 필드를 인자로 넣어주는게 꽤 괜찮다는 생각이 드네요.

구체적으로 Order는 처음 생성할 때, OrderStatus.COOKING으로 반드시 만들어져야 하니까 Order.createCooking()이나 Order.cooking() 이라는 static factor method를 만들어줄만하다고 생각해요.
(명시적이고, 생성자 인자를 하나 덜 넣어줘도 되니까 편하겠죠?)

만약 DTO의 toEntity 메소드를 통하지 않고 바로 객체에서 생성한다면 이런 부분을 명시적으로 service layer에서 코드로 볼 수 있다는게 첫번째 장점

두번째는 코드를 보다가 toEntity 메소드를 확인하기 위해, Service layer에서 OrderRequest -> Order 이런 식으로 생성 로직을 확인하는데, 알아야하는 정보가 더 많아야한다는 점이 걸린다고 느껴져요.

물론 코드가 간단할 땐 큰 문제가 아니지만, 복잡해질 수록 이런 부분에서 하나 하나씩 비용이 많아지는게 부담일 수 있다고 느껴져요.

그리고 덤으로 DTO에 검증 책임을 하는 것이 어떠냐는 의견을 주셨는데(이번 단계에선 그렇게 하지 않으셨지만), 저는 반드시라는 것은 없지만 99% 도메인 객체에서 수행 할 것 같아요. 이유는 다음과 같습니다.

대부분의 사람이 DTO보다 도메인 객체를 많이 볼 것이에요(단위 테스트를 포함해서). 그리고 객체에 생성에 관련된 검증 로직은 비즈니스 룰 상 중요하기 때문에 도메인 객체에 가는 것이 맞다고 느껴져요.

DTO랑 도메인 객체랑 변경되는 사이클이 다를 거에요. 근데 같은 곳에 몰아 넣는 다면 변경이 2가지 이유에 대해서 이루어질것이에요.

비즈니스 룰 상으로 존재해서는 안되는 도메인 객체가 생성 될 수 있어요. 물론 DTO를 통해서만 생성한다면 그에 대한 리스크가 줄 겠지만 그런 맥락을 알아야한다는 것이 비용이라고 생각해요.

저라면 null이나 가격, 수량들의 음수 체크정도만 DTO에서 해줄 것 같아요.

그런데 규칙이 변할 수 있는 가능성은 염두에 두지 않아도 되는 걸까요?
만약, 테이블이 생길 때 함께 상태를 결정시키는 경우로 변경되게 된다면 수정이 필요할 텐데 당장의 규칙에만 만족할 수 있는 경우를 해줘야 할지가 의문입니다.
그런데 그렇게 된다면 실수는 확실히 방지할 수 있을 것 같아 좋네요!

그리고 DTO에 대한 의견도 감사합니다!
저 역시 가능하다면 둘 모두에서 하되 시간이 없다면 도메인에서 최대한 진행할 것 같네요.

그리고 원래 dto 검증에 대해 질문의 의도는 dto 내부에 아래와 같이 존재하는 코드들도 도메인으로 넘길지에 대한 의문이긴 했습니다!

예를 들어 TableGroupService에서 create를 수행하는 경우 원래 기존에는 orderTables의 개수를 서비스에서 진행해 주었습니다.

if (CollectionUtils.isEmpty(orderTables) || orderTables.size() < 2) { throw new IllegalArgumentException(); }

그런데 요청에 대해 dto로 변경하더라도 이 dto의 List<Long> orderTables로 검증이 가능할 것 같은데 이에 대해서 어떻게 처리하실 것인지 궁금했습니다.
이건 아마 비용적인 문제가 될 것 같은데(n+1 문제도 해결하지 않아 어불성설이긴 하지만…), dto 값을 통해 검증한다면 굳이 쿼리 비용을 발생시키지 않고도 검증이 가능할 것입니다.
비즈니스 룰에 부합하지 않다면 아무 쿼리 요청 없이 예외를 반환해 종료시킬 수 있는 것이죠!
그랬을 때 이 비용을 고려하는 것이 적합한 것인지에 대한 의문이 들어 여쭤보게 되었습니다.

전 쿼리를 안날리고 처리한다는게 어떻게 가능한지 잘 이해가 가지 않네용 ?_?
어쨋든 List<OrderTable>을 찾는 쿼리가 필요하지 않나요?

제가 예시로 올린 코드의 경우 리스트 내용이 존재하는지와 크기만 중요한 것이기에 꼭 쿼리를 통해 조회한 객체가 필요하다고 생각하지 않습니다.
request 요청을 통해 받은 값을 먼저 검증하니까요!
그래서 이런 경우도 과연 쿼리 요청이라는 비용을 감수한 후에 진행하는 것이 좋은 가에 대한 의문이었습니다.

carsago · 2023-10-25T11:02:15Z

src/main/java/kitchenpos/application/OrderService.java

+ validateOrderStatus(order);
+ order.updateOrderStatus(OrderStatus.valueOf(changeStatusRequest.getOrderStatus()));


이 로직은 Order 객체에게 책임을 위임할 수 있겠네요!

그렇네요!
덕분에 더 깔끔해졌습니다 👍

carsago · 2023-10-25T11:05:09Z

src/main/java/kitchenpos/domain/OrderTable.java

+ public void updateNumberOfGuests(final int numberOfGuests) {
+ validateNumberOfGuests(numberOfGuests);
+ this.numberOfGuests = numberOfGuests;
 }


이 검증은 객체가 생성될 때도 필요할 것 같네요 !!

해당 부분을 간과했군요…!
생성 시에도 검증하도록 수정했습니다.

carsago · 2023-10-25T11:06:35Z

src/main/java/kitchenpos/application/TableService.java

+ if (Objects.nonNull(orderTable.getTableGroup())) {
 throw new IllegalArgumentException();
 }


이런 부분도 예외 메시지를 넣어주면 리팩터링을 적극적으로 할 때 좀 더 편할 것 같네용

앗… 한다고 했는데 예외 메시지 처리를 하지 않은 게 꽤나 있었군요..!
감사합니다!

그런데 이건 좀 다른 이야기이긴 한데, 오리의 경우 예외 메시지를 어디서 작성하시나요?

현재 도메인, 서비스할 것 없이 예외 메시지를 예외를 생성한 곳에서 작성하고 있습니다.
그런데 도메인에서 과연 현재처럼 자세한 예외 메시지를 알아야 할까요?

프로젝트를 진행할 때의 경우 저희 팀은 예외 메시지에 대해 도메인이 알 필요가 없으며, 상황에 따라 다른 예외 메시지를 반환할 수 도 있기에 예외 발생의 역할이 도메인이라고 생각하지 않았었습니다.
그래서 값객체 외의 대부분의 검증을 서비스에서 수행해 주었습니다.

혹시 이에 대해 오리는 어떻게 생각하시는지 궁금합니다!

프로젝트라면 한 곳에서 모았겠지만, 미션은 귀찮아서 그냥 예외 생성자에 바로 적었습니다.
그리고 예외를 모두 타입 구분시킨다면 문제가 없겠지만, 그렇지 않으면 테스트코드에서 "정말 내가 예상한 부분에서 발생한 예외로 인해서 테스트가 통과한 것이 맞나?" 라는 의문이 들어서용

carsago · 2023-10-25T11:15:06Z

src/test/resources/truncate.sql

+SET REFERENTIAL_INTEGRITY FALSE;
+truncate table menu;
+truncate table menu_group;
+truncate table menu_product;
+truncate table order_line_item;
+truncate table orders;
+truncate table order_table;
+truncate table product;
+truncate table table_group;
+SET REFERENTIAL_INTEGRITY TRUE;


테이블이 추가 혹은 삭제되더라도 sql 변경 엊ㅅ이 원활하게 db 상태를 비워줄 수 있는 방법을 고민해보시는 것도 좋다고 느껴집니다!

AbstractTestExecutionListener의 beforeTestMethod를 재정의 해주어 진행해보았습니다!
혹시 이러한 걸 원하신 것이 맞을까요?

여러 방법이 있겠죠? 제이미가 택하신 것도 하나의 방법이죠~

carsago · 2023-10-25T11:16:46Z

src/main/java/kitchenpos/application/TableService.java

+ validateOrderTableIsEmpty(savedOrderTable);
+
+ savedOrderTable.updateNumberOfGuests(numberOfGuests);


이 부분도 객체에게 메시지를 보낼 수 있겠네요

그렇네요!
덕분에 더 깔끔해졌습니다 👍 22

JJ503 · 2023-10-26T08:16:04Z

오리 좋은 피드백들 감사합니다!
답변 및 수정을 진행해 보았는데 확인해 주시면 감사하겠습니다.

그리고 추가적으로 생각해 보니 1단계 피드백에 대한 답변을 노션에 적어두고 실제 답변으로는 달지 않았더라고요.. 😅
1단계 미션에 대한 피드백 답변 및 추가적인 질문에 대해 작성해보았습니다.
이 역시 함께 확인해주시면 감사하겠습니다! (1단계 pr 바로가기)

carsago

제이미 이번 단계는 이만 merge 하겠습니다 3단계에서 뵙겠습니다

carsago · 2023-10-26T10:29:51Z

src/main/java/kitchenpos/application/TableService.java

+ if (Objects.nonNull(orderTable.getTableGroup())) {
 throw new IllegalArgumentException();
 }


프로젝트라면 한 곳에서 모았겠지만, 미션은 귀찮아서 그냥 예외 생성자에 바로 적었습니다.
그리고 예외를 모두 타입 구분시킨다면 문제가 없겠지만, 그렇지 않으면 테스트코드에서 "정말 내가 예상한 부분에서 발생한 예외로 인해서 테스트가 통과한 것이 맞나?" 라는 의문이 들어서용

carsago · 2023-10-26T10:29:57Z

src/main/java/kitchenpos/application/OrderService.java

+ orderTableRepository.findById(orderRequest.getOrderTableId())
+ .orElseThrow(() -> new NotFoundOrderTableException("해당 주문 테이블이 존재하지 않습니다."));
+ final List<OrderLineItem> orderLineItems = convertToOrderLineItem(orderRequest);
+ final Order order = orderRequest.toEntity(orderTable, OrderStatus.COOKING, LocalDateTime.now(), orderLineItems);


전 쿼리를 안날리고 처리한다는게 어떻게 가능한지 잘 이해가 가지 않네용 ?_?
어쨋든 List<OrderTable>을 찾는 쿼리가 필요하지 않나요?

carsago · 2023-10-26T10:31:00Z

src/test/resources/truncate.sql

+SET REFERENTIAL_INTEGRITY FALSE;
+truncate table menu;
+truncate table menu_group;
+truncate table menu_product;
+truncate table order_line_item;
+truncate table orders;
+truncate table order_table;
+truncate table product;
+truncate table table_group;
+SET REFERENTIAL_INTEGRITY TRUE;


여러 방법이 있겠죠? 제이미가 택하신 것도 하나의 방법이죠~

carsago · 2023-10-27T01:08:21Z

src/test/java/kitchenpos/application/OrderServiceTest.java

+ final Order order = orderRepository.save(OrderFixture.조리_상태의_주문_엔티티_생성(orderTable, menu));
+ final ChangeOrderStatusRequest changeStatusRequest = new ChangeOrderStatusRequest(OrderStatus.MEAL.name());


https://openjdk.org/projects/amber/guides/lvti-style-guide

저는 말씀드린것 처럼 같은 라인에서 생성자를 호출할 때 선호하는 편이에요.
가독성이 이라는게 주관적이라지만, 역설적으로 동시에 보편적인 기준이기도 해요. 디자인등을 봐도 그렇죠.

JJ503 added 30 commits October 19, 2023 06:23

test: 가독성을 위한 개행 수정

c824b8d

test: 주문 상태 변경 테스트 추가

6592128

refactor: product setter 제거

015723d

refactor: Menu setter 제거

565300b

refactor: MenuGroup setter 제거

a9fa778

refactor: MenuProduct setter 제거

f447a18

chore: jpa ddl 설정 추가

c250762

refactor: 모든 도메인에 jpa entity 적용

bb339b7

refactor: 모든 도메인에 jpa entity 적용 및 레파지토리 생성

8624397

refactor: 서비스에서 ProductRepository를 사용하도록 수정

06c3358

refactor: 서비스에서 MenuRepository를 사용하도록 수정

2782e1e

refactor: 서비스에서 MenuRepository를 사용하도록 수정

89e39da

refactor: 서비스에서 MenuRepository를 사용하도록 수정

269a2a7

refactor: 서비스에서 MenuRepository를 사용하도록 수정

c11f578

refactor: 서비스에서 OrderTableRepository를 사용하도록 수정

4ff0291

refactor: 서비스에서 TableGroupRepository를 사용하도록 수정

7df9ea5

refactor: 서비스에서 OrderRepository를 사용하도록 수정

19f306b

refactor: 서비스에서 MenuProductRepository와 OrderLineItemRepository를 사용하도록 수정

f3d7b31

refactor: Menu 동등성 재정의 및 toString 추가, MenuServiceTest 코드 리팩터링

fecc9cf

refactor: Product 동등성 재정의 및 toString 추가, ProductServiceTest 코드 리팩터링

ef5ce5f

refactor: Menu 동등성 재정의 및 toString 추가, MenuServiceTest 코드 리팩터링

77955f6

refactor: MenuProduct 동등성 재정의 및 toString 추가

6c627bc

refactor: Order 동등성 재정의 및 toString 추가, OrderServiceTest 코드 리팩터링

621b333

refactor: OrderLineItem 동등성 재정의 및 toString 추가

f8be514

refactor: OrderTable 동등성 재정의 및 toString 추가, TableServiceTest 코드 리팩터링

2acae49

refactor: 잘못된 동등성 재정의 수정

a09c2fb

refactor: TableGroup 동등성 재정의 및 toString 추가, TableGroupServiceTest 코드 …

f30ef65

…리팩터링

feat: MenuController에 요청과 응답 dto 추가

86677fd

refactor: 불필요한 테스트 fixture 파일 제거

f709327

feat: ProductController에 요청과 응답 dto 추가

0942c44

JJ503 added 10 commits October 25, 2023 04:01

feat: MenuGroupController에 요청과 응답 dto 추가

3dbe32f

feat: TableController에 요청과 응답 dto 추가

732b103

feat: TableGroupController에 요청과 응답 dto 추가

d4bb0f9

feat: OrderController에 요청과 응답 dto 추가

8081211

refactor: price 검증 로직 분리

68ef258

refactor: MenuService 로직 분리

d795f36

refactor: TableGroupService 로직 분리

2bf014b

refactor: TableService 로직 분리

8844396

refactor: OrderService 로직 분리

0dc74ee

refactor: setter 제거

2e3c5ad

JJ503 self-assigned this Oct 25, 2023

JJ503 added 5 commits October 25, 2023 16:09

refactor: price의 동등성 재정의

3a8d6c1

refactor: transactional readonly 속성 추가

d70caab

refactor: usingRecursiveComparison() 사용 제거

be23d59

refactor: Order에서 OrderStatus를 사용하도록 수정

b27428c

refactor: 변수 네이밍 수정

6ed3c8a

carsago requested changes Oct 25, 2023

View reviewed changes

JJ503 added 8 commits October 26, 2023 11:05

refactor: 테스트를 위한 생성자 제거

dc123ef

refactor: 정적팩토리메서드로 객체를 생성하는 도메인에 대한 생성자의 접근 제어자를 private으로 수정

7a0b990

refactor: 불필요한 static 제거

176edb9

refactor: 서비스와 도메인 로직 분리

a9fce68

refactor: 주문 테이블 생성 시 인원수 검증 로직 추가

b03badf

refactor: 기본 예외를 커스텀 예외로 구분 및 예외 메시지 추가

dc7576e

refactor: 가독성을 위한 개행 및 메서드 분리 수정

4d6e562

refactor: 가독성을 위한 개행 및 메서드 분리 수정

db89e08

carsago approved these changes Oct 27, 2023

View reviewed changes

carsago merged commit 86df1d0 into woowacourse:jj503 Oct 27, 2023

		final Order order = orderRepository.save(OrderFixture.조리_상태의_주문_엔티티_생성(orderTable, menu));
		final ChangeOrderStatusRequest changeStatusRequest = new ChangeOrderStatusRequest(OrderStatus.MEAL.name());

		validateOrderStatus(order);
		order.updateOrderStatus(OrderStatus.valueOf(changeStatusRequest.getOrderStatus()));

		validateOrderTableIsEmpty(savedOrderTable);

		savedOrderTable.updateNumberOfGuests(numberOfGuests);

[레거시 코드 리팩터링 - 2단계] 제이미(임정수) 미션 제출합니다. #657

[레거시 코드 리팩터링 - 2단계] 제이미(임정수) 미션 제출합니다. #657

Conversation

JJ503 commented Oct 25, 2023 • edited Loading

carsago left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carsago Oct 25, 2023 • edited Loading

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

carsago Oct 25, 2023 • edited Loading

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

carsago Oct 25, 2023 • edited Loading

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

JJ503 commented Oct 26, 2023

carsago left a comment • edited Loading

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

JJ503 commented Oct 25, 2023 •

edited

Loading

carsago Oct 25, 2023 •

edited

Loading

carsago Oct 25, 2023 •

edited

Loading

carsago Oct 25, 2023 •

edited

Loading

carsago left a comment •

edited

Loading