[레거시 코드 리팩터링 - 2단계] 다니(이다은) 미션 제출합니다. #193
Conversation
| Long quantity = menuProduct.getQuantity(); | ||
| totalPrice = totalPrice.add(price.multiply(BigDecimal.valueOf(quantity))); |
There was a problem hiding this comment.
MenuProduct의 금액 계산시 MenuProduct에서 quantity를 가져와서 계산하지 않고, MenuProduct에 메세지를 보내는 건 어떻게 생각하시나요?
|
|
||
| public Long getId() { | ||
| return id; | ||
| private Long numberOfGuests; |
There was a problem hiding this comment.
이 부분을 Long으로 바꾸신 이유가 있을까요?? clone을 받아 실행시켜보려고 했는데 이 부분에서 문제가 있어서 실행이 안되는 것 같네요ㅠㅠ
| Orders orders = ordersRepository.findById(requestDto.getOrderTableId()) | ||
| .orElseThrow(IllegalArgumentException::new); |
There was a problem hiding this comment.
Order을 조회하는데 OrderTableId를 사용해서 조회하고 있어요!
해당 Table의 모든 Order을 검증하기 위해서는 List 형태로 가져와야 할 것 같아요!
OrderTable 내부에서 List를 객체 참조해도 될 것 같고, 그게 아니라면 orderTableId로 findAllByOrderTable_Id()로 가져와야 할 것 같아요.
| String orderStatus = orders.getOrderStatus(); | ||
| if (COOKING.name().equals(orderStatus) || MEAL.name().equals(orderStatus)) { | ||
| throw new IllegalArgumentException(); | ||
| } |
|
|
||
| return orderTableDao.save(savedOrderTable); | ||
| entityManager.flush(); |
There was a problem hiding this comment.
EntityManager을 주입받아서 flush 해주신 이유가 있을까요? 이 메서드 말고 전체적으로 EntityManager을 주입받아 사용하신 부분이 있던데 이유가 궁금하네요!
| List<OrderTable> orderTables = tableRepository | ||
| .findAllByTableGroupId(requestDto.getId()); | ||
|
|
||
| if (ordersRepository.existsByOrderTableInAndOrderStatusIn( | ||
| orderTables, | ||
| Arrays.asList(COOKING.name(), MEAL.name()))) { | ||
| throw new IllegalArgumentException(); | ||
| } |
There was a problem hiding this comment.
TableGroup에서 List를 참조하고 있어서 TableGroup 조회시 연관된 OrderTable도 가져와질 것 같은데 따로 repository에서 가져오신 이유가 있을까요??
검증도 TableGroup 내에서 메세지를 보내는 식으로 처리할 수도 있을 것 같네요!
| public TableEmptyResponseDto changeEmpty(TableEmptyRequestDto requestDto) { | ||
| OrderTable orderTable = tableRepository.findById(requestDto.getOrderTableId()) | ||
| .orElseThrow(IllegalArgumentException::new); | ||
| if (Objects.nonNull(orderTable.getTableGroup())) { |
| import kitchenpos.application.dto.response.MenuResponseDto; | ||
| import kitchenpos.domain.Menu; | ||
|
|
||
| public class MenuDtoAssembler { |
There was a problem hiding this comment.
Dto -> 도메인이나, 도메인 -> Dto 변환시 전반적으로 Assembler 클래스를 만드셔서 사용하신 걸 봤어요!
혹시 이렇게 따로 클래스를 만들어 사용하신 이유가 있을까요? 처음 보는 방식이라서 어떤 점 때문에 이 방식을 사용하게 되셨는지 궁금해요! 😄
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| private String orderStatus; |
| @@ -8,7 +8,7 @@ CREATE TABLE orders ( | |||
|
|
|||
| CREATE TABLE order_line_item ( | |||
| seq BIGINT(20) NOT NULL AUTO_INCREMENT, | |||
| order_id BIGINT(20) NOT NULL, | |||
| orders_id BIGINT(20), | |||
There was a problem hiding this comment.
이 부분이랑 menu_product의 menu_id 부분의 NOT NULL을 제거하신 이유가 있나요??
각각 실행했을 때(OrdersService#changeOrderStatus())
전체로 실행했을 때(OrdersService#changeOrderStatus())
|
오랜만이에요 에어 ~!
좀 더 빨리 제출하고 싶었는데,, 생각보다 오래 걸렸네요 🥲
2단계는 아래와 같이 진행했습니다 !
지금 changeEmpty()와 관련된 테스트가 각각 실행하면 통과하나, 전체로 실행하면 실패해요 😥
아직 이유를 못 찾고 있는데, 먼저 제출하고 더 알아볼게요!
혹시,, 이번에도 함께 고민해줄 수 있을까요 ㅎㅎㅎㅎ
1단계에서 정성어린 리뷰들 달아줘서 정말 고마웠습니다 !!
이번에도 잘 부탁드립니다 🙇♀️✨