[2단계 - 주문 기능 구현] 성하(김성훈) 미션 제출합니다. #36
Conversation
Co-authored-by: sh111-coder <ohk9134@naver.com>
Co-authored-by: sh111-coder <ohk9134@naver.com>
Co-authored-by: sh111-coder <ohk9134@naver.com>
Co-authored-by: 70825 <hello70825@gmail.com>
Co-authored-by: 70825 <hello70825@gmail.com>
Co-authored-by: 70825 <hello70825@gmail.com>
Co-authored-by: 70825 <hello70825@gmail.com>
Co-authored-by: 70825 <hello70825@gmail.com>
Co-authored-by: 70825 <hello70825@gmail.com>
Co-authored-by: 70825 <hello70825@gmail.com>
| public Quantity changeQuantity(int quantity) { | ||
| return new Quantity(quantity); | ||
| } | ||
|
|
||
| public Quantity addQuantity(int quantity) { | ||
| return new Quantity(this.quantity + quantity); | ||
| } |
There was a problem hiding this comment.
새로 인스턴스를 만드는것도 좋지만.
내부에서 상태를 변경하는게 어떨까요?
굳이 불변을 무조건적으로 유지할필요는 없습니다.
인스턴스를 새로 만들면 그만큼 성능문제도 있구요.
There was a problem hiding this comment.
고민했던 부분이었는데 집어주셔서 감사합니다!
VO는 비즈니스 로직을 가지지 않는 값 객체라서 상태가 변하는 것이 불변을 만들었을 때의 부작용보다 크다면
불변을 유지하지 않아도 괜찮다고 생각합니다!!
그래서 상태가 변하도록 리팩토링 했습니다!
그런데 궁금한 점이 현재 Member 도메인에도 Cash 충전, 결제 시에 상태가 변해야 하는데
이때는 비즈니스 로직이 들어가고 중요한 도메인인만큼 불변을 유지해야한다고 생각해서
Cash 충전, 결제 시 Member 인스턴스를 새로 생성하면서 불변을 유지하고 있는데
이때도 불변 유지 시의 부작용이 더 크다고 생각하실까요?
There was a problem hiding this comment.
네. 굳이 성능과 로직이라고 한다면 전 성능을 선택할거 같습니다.
그러나 지금처럼 매 스레드마다 1회 정도라면 나을수도 있겟죠.
그렇다고 한들 인스턴스를 굳이 새로 만들진 않을거 같아요~
src/main/java/cart/domain/cartitem/presentation/CartItemApiController.java
Show resolved
Hide resolved
src/main/java/cart/domain/product/application/ProductService.java
Outdated
Show resolved
Hide resolved
|
안녕하세요! 럿고! 리팩토링하면서 생겼던 질문 몇 가지를 코멘트 단에 남겨놓았습니다!! |
| @@ -10,7 +10,7 @@ public CartItemException(String message) { | |||
|
|
|||
| public static class IllegalMember extends CartItemException { | |||
| public IllegalMember(CartItem cartItem, Member member) { | |||
| if (cartItem.isEmpty()) { | ||
| return ProductCartItemResponse.createOnlyProduct(findProduct); | ||
| } | ||
| return ProductCartItemResponse.createContainsCartItem(findProduct, cartItem.get()); |
There was a problem hiding this comment.
| if (cartItem.isEmpty()) { | |
| return ProductCartItemResponse.createOnlyProduct(findProduct); | |
| } | |
| return ProductCartItemResponse.createContainsCartItem(findProduct, cartItem.get()); | |
| return cartItem.map(item -> ProductCartItemResponse.createContainsCartItem(findProduct, item)) | |
| .orElseGet(() -> ProductCartItemResponse.createOnlyProduct(findProduct)); |
이렇게 쓰면 깔끔하지 않을까요?
| public List<CartItemResponse> findByMember(AuthMember authMember) { | ||
| Member findMember = memberDao.selectMemberByEmail(authMember.getEmail()); | ||
| List<CartItem> cartItems = cartItemDao.findDescByMemberId(findMember.getId()); | ||
| return cartItems.stream().map(CartItemResponse::from).collect(Collectors.toList()); |
There was a problem hiding this comment.
| return cartItems.stream().map(CartItemResponse::from).collect(Collectors.toList()); | |
| return cartItems.stream() | |
| .map(CartItemResponse::from) | |
| .collect(Collectors.toList()); |
stream은 한줄의 하나씩 쓰는게 깔끔해 보이더라구요 :)
| import cart.domain.order.dto.OrderResponse; | ||
| import cart.domain.order.dto.OrderedProductDto; | ||
|
|
||
| public class OrderItems { |
There was a problem hiding this comment.
음... 그러나 OrderItem의 상태중 하나가 Order이기 때문에 객체지향적으로도 어색하진 않아 보여요!
| import cart.domain.cartitem.domain.CartItem; | ||
| import cart.domain.product.domain.Product; | ||
|
|
||
| public class ProductCartItemResponse { |
There was a problem hiding this comment.
음... 뭐랄까... 어색하지 않다고 생각한다고 이야기 드리는거 같아요.
굳이 API를 두번 호출할 필요가 없다면 괜찮을거 같아요. 리소스상에서 어색하지도 않구요.
대신 이럴수가 있죠. 장바구니 정보를 따로 내려줘야 하는 API를 만들어야 한다면 분리할수도 있겠죠.
이건 좀 관점에 차이인데요 true,false만 필요하다면 true, false만 내려주는게 좋을거 같습니다.
프론트에서 데이터를 내려줄때 필요한 데이터만 내려줘야 한다고 생각합니다. 과하게 데이터를 내려줄 필요가 없다고 생각해요. 에를들어 product의 name이 필요없는데 차후를 위해 내려주자. 이것보단 그냥 차후 필요할떄 추가하는게 더 좋다고 생각해요.
안녕하세요 럿고! 이번 미션 함께하게 된 리뷰이 성하입니다!! 😃
이번 미션 커밋 범위는 아래와 같습니다!
커밋 범위
이번에는 안드로이드와 협업하면서 API 문서를 RestDocs로 만들어보게 되었습니다!
API 문서
🎯 구현 기능
해당 요구 사항 중에서 '포인트'를 구현해보았습니다.
저희 애플리케이션에서 '포인트'는 적립되는 포인트가 아닌,
재화로서의 '포인트'로, 충전하여 실제로 주문 시 가진 포인트로 결제되도록 구현하였습니다.
🎯 시나리오
이번 구현에서는 배운 ATDD를 적용해서 먼저 기능 목록에 시나리오를 작성하고, 그를 기반으로 ATDD를 진행하였습니다.
확실히 ATDD로 하니 길을 잃어도 금방 방향을 찾기 쉬웠던 것 같습니다!
대신에 지금 테스트를 180개 가량을 짰는데, 그로 인해 구현 속도가 좀 느렸던 것 같네요 😂
처음 애플리케이션 코드를 보실 때도 시나리오 기반으로 이해하면 좋을 것 같아 README 링크를 같이 첨부합니다!!
사용자 시나리오 README
🎯 리팩토링 방향
이번 코드는 빠르게 구현하는 것에 초점을 두고 구현을 완료하고 바로 제출하게 되었습니다!
그래서 리뷰 요청 이후에 다음과 같은 방향으로 리팩토링을 진행하고자 합니다!
🎯 구현 질문들
구현하면서 생긴 질문 사항들은 코드 단에 코멘트로 남겨놓도록 하겠습니다!!
잘부탁드립니다! 럿고!! 😃