[주문] 택이 미션 제출합니다. #2
Conversation
| OrderValidator orderValidator = new OrderValidator(); | ||
|
|
||
| OrderFactory orderFactory = new OrderFactory(); | ||
| OrderService orderService = new OrderService(orderValidator, orderFactory); |
There was a problem hiding this comment.
- 필드에 접근제어자를 지정해주는 건 어떤가요?
- 초기화는 생성자에서 해주는 건 어떨까요?
There was a problem hiding this comment.
사실 이 부분에 대해서 불확실한 느낌을 받았습니다. 피드백 감사합니다. 👍
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public class ProductConverter { |
There was a problem hiding this comment.
dto 변환하는 작업을 클래스로 따로 빼서 해주는 이유가 궁금해요~
There was a problem hiding this comment.
View 에서 입력 값을 변환하는 로직을 같이 넣어두면 클래스 안에 많은 요소가 생길 거 같아. 따로 분리해서 빼뒀습니다! 🤔
| this.orderFactory = orderFactory; | ||
| } | ||
|
|
||
| public OrderResponseDTO getOrderResponseDTO(List<OrderRequestDTO> orderRequestDTOS) { |
There was a problem hiding this comment.
메서드 네이밍을 좀 더 고민해보는 건 어떨까요?
dto 는 response 형태일뿐이고, 이 메서드가 어떤 역할을 하는지 더 잘 담아볼 수 있을 것 같아요~
There was a problem hiding this comment.
흠,,! 더욱 고민해 보겠습니다. 확실히 중심 로직인데 네이밍에서 와닿는 점이 부족한 거 같아요..
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| private OrderRequestDTO createOrderRequestDTO(String productNameString) { |
There was a problem hiding this comment.
| private OrderRequestDTO createOrderRequestDTO(String productNameString) { | |
| private OrderRequestDTO createOrderRequestDTO(String productNameString) { |
| DUMPLING("서비스 만두", 0, SERVICE); | ||
|
|
||
| private final String name; | ||
| private final Integer price; |
There was a problem hiding this comment.
int를 Integer로 감싼 이유를 생각해보니, 확 답변할 수 가 없었습니다.
단지, 여러 메서드를 사용할 수 있고, Null 값 방지로만 생각하고 쓴 거 같습니다.
이 점 다시 한번 학습해서 고쳐보도록 하겠습니다. 🙇🏻
|
|
||
| private OrderServiceResponseDTO createOrderServiceResponseDTO(Integer countMainTypeProduct) { | ||
| if (countMainTypeProduct == 0) | ||
| return null; |
There was a problem hiding this comment.
null 반환 하면 View 전달되어서 null 로 판단 시 서비스를 제외하는 방안으로 했습니다. 하지만, null 반환은 좋은 방법은 아닌 거 같아서 다시 고민해 보겠습니다.
| @@ -0,0 +1,8 @@ | |||
| package order.dto; | |||
There was a problem hiding this comment.
다시 생각해보니, 이유는 크게 없는 거 같습니다,, 패키지 분리 작업 하면서 자연스럽게 order 밑으로 선언한 거 같아요. 패키지 정리 작업하면서 다시 해보겠습니다!
| public void validateProductQuantity(Order product) { | ||
| int quantity = product.getQuantity(); | ||
|
|
||
| if (quantity < 0 || quantity > 10) |
There was a problem hiding this comment.
이쪽 클래스는 분리를 완전히 못했던 거 같습니다. 다시 한번 생각해서 분리 해보겠습니다. 감사합니다.
| } | ||
|
|
||
| public void validateOnlyDrinks(List<Order> orders) { | ||
| boolean allMatch = orders.stream() |
There was a problem hiding this comment.
반환값이 boolean 일땐 is~ 또는 has~ 의 네이밍을 사용해야 파악이 쉬울 것 같아요~
There was a problem hiding this comment.
아하 🫢 변수도 네이밍 하는 걸 깜빡한 거 같습니다. 감사합니다.
| @@ -0,0 +1,32 @@ | |||
| package order.constant; | |||
There was a problem hiding this comment.
9개,, 패키지를 정리해서 옮기겠습니다! 감사합니다.
정말.. 감사합니다. 따끔한 리뷰가 저한테는 다시 되돌아보는 큰 기회가 되었습니다. 남겨주신 피드백 내용 적극 반영해서 추후 제출에 개선된 코드로 다시 찾아뵙겠습니다. |
안녕하세요! 좋은 미션 제공해 주셔서 정말 감사합니다.
코드 리뷰 중점 🚀
이외에도 다양하게 봐주시면 정말 감사드립니다! 🙇🏻