[2단계 - 주문 기능 구현] 다즐(최우창) 미션 제출합니다. #28
Conversation
| List<CartItem> cartItems = cartItemDao.findByMemberId(member.getId()); | ||
| return cartItems.stream().map(CartItemResponse::of).collect(Collectors.toList()); | ||
| List<CartItem> cartItems = cartItemRepository.findByMemberId(member.getId()); | ||
| return cartItems.stream() |
There was a problem hiding this comment.
CartItemResponse에 List<CartItem> 을 받아 List<CartItemResponse> 를 만드는 정적 팩터리 메서드를 만들어주어도 괜찮을것 같네요 (다른 List<~Reponse> 에도 적용 가능할 것 같아요)
There was a problem hiding this comment.
말씀해주신대로 DTO 생성 책임을 온전히 DTO로 넘기게 되었더니 서비스 코드가 더욱 깔끔해진 것 같습니다 👍
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public CartItemPriceResponse getTotalPriceWithDeliveryFee(Member member, List<Long> cartItemIds) { |
There was a problem hiding this comment.
find~ 와 get~ 의 차이가 있나요?
의도를 갖고 메서드명을 구별하신것인지 궁금합니다 :)
There was a problem hiding this comment.
기존에 제공하는 메서드들을 그대로 활용했던 것도 존재하기에 특별한 기준없이 혼용되어 있는 부분도 존재합니다 😅
저의 기준으로는 단순히 조회를 통해 클라이언트가 원하는 형식의 응답값을 반환하는 역할을 수행한다면 find를 사용하고, 조회를 통해 얻은 데이터를 바탕으로 비즈니스 로직에 알맞게 값을 생성한 경우에는 get을 사용하고자 합니다.
이 부분에 있어 코즈도 기준이 존재하는지 궁금합니다!
There was a problem hiding this comment.
전 find라는 말 자체가 존재하지 않을수도 있다는 말을 내포한다고 생각해서요. List나 Optional을 반환하는 경우엔 find, 그 외엔 get을 사용합니다.
�이 방법이 좋다, 추천한다는건 아니고 그냥 분리가 되어있길래 질문드렸었어요 :)
| } | ||
|
|
||
| private int calculateTotalPrice(List<CartItem> cartItems) { |
There was a problem hiding this comment.
List<CartItem> 을 멤버 변수로 가진 객체 (일급 컬렉션)을 만들면 여러 로직을 도메인 객체에게 위임할 수 있을것 같네요 (checkOwner 등)
There was a problem hiding this comment.
CartItems 도메인을 생성하여 서비스의 로직을 도메인 객체에 위임하도록 수정하였습니다! 이렇게 함으로 서비스에서는 도메인간의 비즈니스 규칙에 집중할 수 있었고 도메인에 로직이 존재하므로 재사용과 테스트가 쉬워지는 것 같습니다 🙇🏻♂️
| cartItem.checkOwner(member); | ||
| } | ||
| Order order = new Order(member, orderInCart(cartItems), orderRequest.getPoint()); | ||
| if (order.getTotalPrice() < orderRequest.getPoint()) { |
There was a problem hiding this comment.
getter 를 사용해서 비교하는것이 아니라, 도메인 객체에게 �포인트 비교의 책임을 위임하면 어떨까요?
There was a problem hiding this comment.
도메인의 값과 비교하여 검증이 필요한 상황일 때 도메인 내부에서 예외를 발생시키는 방법과 도메인에서는 boolean으로 검증을 진행하고 외부에서 예외를 발생시키는 방법이 있을 것 같은데 어떤 방법을 사용하시나요? 🤔
There was a problem hiding this comment.
예외를 발생시키는 것이 더 나은것 같습니다 :)
혹시 boolean의 장점이라고 볼 수 있는 부분이 있을까요?
| } | ||
|
|
||
| @Override | ||
| public void addCorsMappings(final CorsRegistry registry) { |
There was a problem hiding this comment.
현재 미션에서 중요한부분은 아닐수 있지만, cors 허용의 대상 또는 메서드를 한정할 수 도 있을것 같아요
| } | ||
| } | ||
|
|
||
| public CartItem changeQuantity(int quantity) { |
There was a problem hiding this comment.
새로운 객체를 반환하는 것은, 불변 객체로서 사용하기 위함인가요?
그냥 quantity만 바꿔도 되지 않나요?
There was a problem hiding this comment.
불변 객체로 사용하려고 했으나, ID를 할당하는 과정에서도 id를 수정하는 작업이 있기에 qantity도 수량만 변경하는 방법으로 수정하였습니다!
| } | ||
|
|
||
| public int getTotalPrice() { | ||
| return product.getPrice() * quantity.getValue(); |
There was a problem hiding this comment.
getter를 사용하는 것이 아니라 연산의 책임을 객체에게 위임할 수 있을것 같습니다 :)
There was a problem hiding this comment.
//CartItem
public int getTotalPrice() {
return product.getPrice(quantity);
}//Product
public int getPrice(Quantity quantity) {
return quantity.calculatePrice(price.getValue));
}//Quantity
public int calculatePrice(int price) {
return price * value;
}도메인에서 사용하는 값 객체의 깊이가 깊은 경우 getter를 최대한 사용하지 않으려고 한다면 위와 같은 코드가 연쇄적으로 나올 것 같아요. 어디서 적절히 끊어주는게 좋을지 궁금합니다!
There was a problem hiding this comment.
�작성해주신 Quantity와 같은 값을 나타내는 객체는 연산에 관련된 메서드 없이도 값 검증에 대한 역할을 합니다, 하지만 연산에 대한 행위 없이 getter만 사용한다면 더 활용할 수 있는 여지가 있는 객체를 충분히 활용하지 못하는 것이라 생각합니다.
예를 들어, 음수를 곱하면 안된다는 요구사항이 발생했을때 (현재는 생성자에서 방어가 되고 있지만, 비슷한 요구사항이 있다고 가정해보죠) 모든 연산에 if ( 음수 ) throw ~ 를 적어주는것이 아니라 연산 메서드 한곳만 수정하면 되는 일이 생길수도 있구요.
이러한 면에서 저는, getter를 사용하지 않는것은 어디서 적절히~ 보단 습관적으로 최대한~ 을 따르고 있는것 같습니다.
가치관에 따라 달라질순 있는 부분이라고 생각합니다 :)
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(new ErrorResponse(e.getMessage())); | ||
| } | ||
|
|
||
| @ExceptionHandler(BadRequestException.class) |
There was a problem hiding this comment.
도메인에서 발생하는 예외의 메세지가 클라이언트의 응답으로 그대로 노출되고 있는것 같은데, 도메인이 뷰로직을 갖고있다고 생각할수 있지 않을까요?
클라이언트가 해당 메세지를 사용자에게 노출하는 상황�이라고 한다면, 문구의 변경이 필요해졌을때 도메인 영역에 변경이 생기지 않나요?
There was a problem hiding this comment.
현재 방식에서는 말씀해주신 것과 같이 문구의 변경이 필요해졌을 때 도메인 영역의 변경이 발생할 것 같습니다. 이를 막기 위해서는 예외 메시지를 예외 안에서 정의하도록 하고 각 예외 상황에 대한 커스텀 예외를 구현하는 방법이 있을 것 같아요!
각각의 예외 상황에 대한 커스텀 예외를 정의하면 예외 메시지가 예외에 정의되어 있기에 문구를 출력하는 상황에서도 도메인 영역의 변화는 발생하지 않습니다. 하지만 모든 예외 상황에서 개별의 커스텀 예외를 생성해야 하기 때문에 커스텀 예외가 무분별하게 증가하는 문제도 발생할 것 같아요. 실제 현업에서는 이러한 예외를 어떻게 관리하고 있나요? 🤔
There was a problem hiding this comment.
현업에서도 공통된 예외를 사용하는 경우는 존재합니다 :)
커스텀 예외가 무분별하게 증가하는 것이 우려가 된다라면, 계층별 예외 클래스를 하나 정의해두고, 멤버변수로 예외 코드같은 상수를 만들어 관리하는 방법도 있을것 같아요.
예외에 대한 처리는 프론트분들과도 맞춰보시기 바랍니다. 클라이언트에 따라문구를 서버에서 관리해야하는 경우도 있습니다. 이땐, 작성해주신 코드 기준으로 ui 계층에서 도메인을 의존하여 문구를 만들어내면, 도메인 영역의 변경을 없앨 수 있을것 같아요
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public CartItemPriceResponse getTotalPriceWithDeliveryFee(Member member, List<Long> cartItemIds) { |
There was a problem hiding this comment.
전 find라는 말 자체가 존재하지 않을수도 있다는 말을 내포한다고 생각해서요. List나 Optional을 반환하는 경우엔 find, 그 외엔 get을 사용합니다.
�이 방법이 좋다, 추천한다는건 아니고 그냥 분리가 되어있길래 질문드렸었어요 :)
| cartItem.checkOwner(member); | ||
| } | ||
| Order order = new Order(member, orderInCart(cartItems), orderRequest.getPoint()); | ||
| if (order.getTotalPrice() < orderRequest.getPoint()) { |
There was a problem hiding this comment.
예외를 발생시키는 것이 더 나은것 같습니다 :)
혹시 boolean의 장점이라고 볼 수 있는 부분이 있을까요?
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(new ErrorResponse(e.getMessage())); | ||
| } | ||
|
|
||
| @ExceptionHandler(BadRequestException.class) |
There was a problem hiding this comment.
현업에서도 공통된 예외를 사용하는 경우는 존재합니다 :)
커스텀 예외가 무분별하게 증가하는 것이 우려가 된다라면, 계층별 예외 클래스를 하나 정의해두고, 멤버변수로 예외 코드같은 상수를 만들어 관리하는 방법도 있을것 같아요.
예외에 대한 처리는 프론트분들과도 맞춰보시기 바랍니다. 클라이언트에 따라문구를 서버에서 관리해야하는 경우도 있습니다. 이땐, 작성해주신 코드 기준으로 ui 계층에서 도메인을 의존하여 문구를 만들어내면, 도메인 영역의 변경을 없앨 수 있을것 같아요
| } | ||
|
|
||
| public int getTotalPrice() { | ||
| return product.getPrice() * quantity.getValue(); |
There was a problem hiding this comment.
�작성해주신 Quantity와 같은 값을 나타내는 객체는 연산에 관련된 메서드 없이도 값 검증에 대한 역할을 합니다, 하지만 연산에 대한 행위 없이 getter만 사용한다면 더 활용할 수 있는 여지가 있는 객체를 충분히 활용하지 못하는 것이라 생각합니다.
예를 들어, 음수를 곱하면 안된다는 요구사항이 발생했을때 (현재는 생성자에서 방어가 되고 있지만, 비슷한 요구사항이 있다고 가정해보죠) 모든 연산에 if ( 음수 ) throw ~ 를 적어주는것이 아니라 연산 메서드 한곳만 수정하면 되는 일이 생길수도 있구요.
이러한 면에서 저는, getter를 사용하지 않는것은 어디서 적절히~ 보단 습관적으로 최대한~ 을 따르고 있는것 같습니다.
가치관에 따라 달라질순 있는 부분이라고 생각합니다 :)
안녕하세요 코즈! 이번 미션 리뷰를 부탁드리게 된 다즐입니다 😀
프론트분들과 협업을 진행하였는데 너무 밝고 재밌는 분위기에서 진행할 수 있어서 좋은 기억으로 남을 것 같네요. 함께했던 백엔드 팀원들과도 개발하면서 발생하는 많은 고민을 같이 얘기하면서 너무 재미있었던 미션이었습니다.
이번 미션에서 중요하다고 생각한 부분은 API 설계, 배포, 인프라, 테스트였던 것 같습니다. 프론트팀에서 저희가 개발하는 API를 사용하기에 설계를 함께 얘기하며 요구사항을 맞추어 나갔고, 애플리케이션을 서버에 배포하는 과정에서 인프라에 대한 학습도 진행할 수 있었습니다. 그리고 외부에서 사용되는 API인만큼 예상하지 못한 에러가 발생하지 않도록 꼼꼼하게 테스트 코드를 작성하려고 노력했습니다.
이번 리뷰 범위입니다! 잘 부탁드리겠습니다 :) 감사합니다 🙇🏻♂️