[2단계 - 상품 관리 기능] 제이(이재윤) 미션 제출합니다. #262
Conversation
There was a problem hiding this comment.
안녕하세요 제이, 리뷰어 시카입니다. 😄
2단계도 요구사항에 맞춰 빠르게 구현해주셨네요! 👍
몇 가지 코멘트 남겨드렸으니 확인 부탁드려요!
추가로 궁금한 점이 생기면 언제든지 DM 주세요 ㅎㅎ
고민 끝에 저는 Repository를 통해서 CartService에서 직접 객체를 가져오는 것이 아닌 다른 Service를 통해서 객체를 가져오는 방식을 택했습니다.
그 이유는 Repository를 통해 직접 가져오는방식을 사용하면 서비스가 커지고, 동일한 객체 호출의 반복이 높아진다면 같은 코드를 다른 Service에서 매번 다 작성해주는데, 그렇게 되면 가져오는 로직이 바뀐다면 모두 다 바꿔야한다고 생각했습니다.
물론 Repository를 통해서 객체를 가져온다면, 조금 더 유연하게 사용할 수 있는 장점은 있는 것 같습니다. (ex. Optional로 서비스마다 다른 예외를 보내주는 것)
시카는 Service에서 다른 Service를 의존하는 것에 대해서 어떻게 생각하시나요?
제이가 고민했던 내용대로 각각의 장단점이 있다는 것과 서비스에서 다른 서비스를 의존하는게 장점이 크다는 것에 동의합니다! 😄
현재 상태에서 Service는 CRUD가 거의 전부이기 때문에 기본적인 CRUD를 서비스 메서드로 사용하고, 필요할 때 추가 로직을 사용해서 처리하면 Repository를 가져오는 것과 비슷한 유연성을 가지고 갈 수 있지 않을까요??
| } | ||
|
|
||
| @GetMapping | ||
| public ResponseEntity<ProductsResponseDto> findMemberCarts(@RequestHeader("Authorization") final String authHeaderValue) { |
There was a problem hiding this comment.
일반적으로 대부분의 기능은 로그인 한 유저에게 제공할텐데요...! 현재와 같은 구조로는
- 모든 컨트롤러 메서드에 중복 코드가 발생
- 비즈니스 로직(여기서는 멤버 카트를 찾아오는)과 관계없는 인증 로직이 들어가는
문제점이 생기게 됩니다. Spring에서 제공하는 HandlerInterceptor, HandlerMethodArgumentResolver 등을 사용해서 개선해보면 어떨까요??
추가로, 클라이언트의 요청이 컨트롤러로 들어오기 전까지의 프로세스도 찾아보면 좋을 것 같아요 ㅎㅎㅎ
There was a problem hiding this comment.
말씀해주신대로 HandlerMethodArgumentResolver를 이용해서 로그인 기능을 처리하니, 중복 로직이 많이 줄었습니다😆
공부를 해보니 인증 로직뿐만 아니라 여러가지 비즈니스 로직과 상관 없는(시간을 측정한다든지.. 등등) 기능들이 중복해서 들어간다면, 이런 것들을 AOP라는 것을 이용해서 처리하기도 하는 것 같습니다!
또한 Filter와 Interceptor에 대해서도 확인을 해보았습니다.
Filter같은 경우에 WebContext에서 디스패처 서블릿으로 가기 전에 작동되고, Interceptor는 디스패처 서블릿을 지나고 컨트롤러로 가기 전에 작동하는 부분이더라고요.
따라서 유저 인증 같은 부분을 컨트롤러로 들어오기 전에 처리해준다면, 조금 더 빠른 인증이 가능하겠네요 :)
감사합니다!
There was a problem hiding this comment.
AOP의 경우는 스프링 핵심 3대 요소라고도 하는데요 ㅎㅎㅎ
스프링 자체도 이러한 것들을 활용해서 구현되어 있으므로 관련된 키워드들이 나올 때마다 내부적으로 어떻게 동작하는지 관심을 가지고 공부하면 더 다양하게 프레임워크를 활용할 수 있지 않을까요??
AOP 이외에 IoC, PSA에 대해서도 이번기회에 가볍게 살펴보고 키워드만 가져가보면 어떨까요??
There was a problem hiding this comment.
좋은 자료 및 키워드 감사합니다 :)
내부 구조 및 동작 원리와 함께 개념들을 함께 공부하면서 익혀나가겠습니다!
| } | ||
|
|
||
| @PostMapping("/{productId}") | ||
| public ResponseEntity<Void> addCart(@PathVariable final Long productId, |
There was a problem hiding this comment.
부연 설명을 드리자면 REST api의 구성요소 중 uniform interface를 키워드로 찾아보시면 좋을 것 같아요!
There was a problem hiding this comment.
void로 유지하고 CREATED status만 리턴하는게 맞다고 생각하신 것 같아요..! ㅎㅎ
그렇다면, 자원의 id라도 return을 해주면 RESTful의 핵심인 클라이언트와 서버 각각의 독립적인 진화에 도움이 될 것 같은데 어떻게 생각하시나요??
There was a problem hiding this comment.
한참 고민했는데 자원을 생성하고, URI를 해당 자원에 대한 식별자로 할당하고, 자원의 기대 값을 반환하라는 말씀이셨군요..!
생각을 해보면, 게시글에 글을 쓰면 제가 작성한 글로 이동하게 만들 수도 있겠네요 :)
감사합니다 😀
| @Autowired | ||
| public ViewController(final ProductService productService, final MemberService memberService) { |
There was a problem hiding this comment.
생성자를 이용해 의존성 주입을 할 때, 생성자가 한 개만 있으면 @Autowired를 생략해도 되지만, 한 개가 아니라서 @Autowired를 선언해주었습니다 :)
There was a problem hiding this comment.
의존성 주입 다양한 방법에 대해서 알고 계시는지 궁금해서 코멘트렸습니다ㅎㅎㅎ
그렇다면 현재는 생성자 주입을 사용하고, 생성자가 1개이니 없애주시면 좋을 것 같아요 😄
| public class Cart { | ||
|
|
||
| private Long id; | ||
| private final Member member; | ||
| private final Product product; |
There was a problem hiding this comment.
도메인 간의 의존 관계를 지금처럼 정하신 이유가 궁금한데 설명해주실 수 있을까요?? ㅎㅎㅎ
일반적인 장바구니를 생각해보면 회원별로 하나의 장바구니를 가지고, 장바구니에 여러개의 상품을 담을 수 있도록 할 것 같아서요!
더 나아가서,
이번 미션에서 사실 "도메인이 필요할까?"라는 생각을 많이 했습니다. 데이터베이스에 저장되는 구조와 같다보니 이런 고민이 들었던 것 같습니다.
지금 도메인은 제이가 말해주신대로 데이터베이스에 저장되는 구조로 사용하고 있기 때문에 이런 고민이 든다고 볼 수 있는데요!
DB와 도메인을 같이 생각하지 않고, 도메인의 행위 먼저 정리해서 정의해보면 어떨까요?? ㅎㅎ
There was a problem hiding this comment.
원래는 Long id, Long memberId, Long productId 이렇게 Entity 형식으로 구성을 하려고 했습니다!
말씀하신대로 Cart도메인의 역할이 없었기 때문이죠!
'도메인'이라는 것에 초점을 맞추려면 제가 작성한 방식보다는 id, 수량, 날짜 등등 정말 장바구니와 관련된 필드들을 가지고, 그를 관리하는 객체가 되어야하는 것이 맞는 것 같습니다.
하지만 지금은 당장 수량, 날짜는 필요 없기 때문에 Entity 형식으로 객체를 바꾸는 것이 맞을 것 같습니다 :)
There was a problem hiding this comment.
id로 간접참조하도록 리팩토링 한 부분은 좋은 것 같아요 👍
추가적으로, 지금은 장바구니라는 도메인 개념의 용어를 다르게 이해하고 있는 것 같아아 같이 맞춰보면 좋을 것 같은데요...!
- 도메인 행위 관점에서 장바구니에 상품을 추가할 때 List carts 에 addCart를 하는 방식(제이 방식)
- 단일 Cart에 Cart.addProduct() 하는 방식
을 비교해서 고민해보면 어떨까요??
|
| public class CartItems { | ||
|
|
||
| private final List<Product> cartItems; |
There was a problem hiding this comment.
도메인으로 먼저 비즈니스 로직(add, delete, contains...)을 처리한 후 이걸 DB로 옮기는 작업을 했는데
여기서 문제가 생겼습니다!
앞 미션에서 하신 것처럼 객체 지향 적으로 설계 부분은 코멘트 드린 의도대로 잘 이해하신 것 같아요 💯💯!!
다만, 여기서 한 번에 일급컬렉션까지 진행하려다 보니, 어려운 부분이 있는 것 같아요 관련된 건 다른 코멘트에서 이어서 말씀드릴게요 😄
| public Cart findCartByMember(final Member member) { | ||
| String sql = "SELECT id FROM cart WHERE member_id = ?"; | ||
| Long cartId = jdbcTemplate.queryForObject(sql, Long.class, member.getId()); | ||
|
|
||
| List<Product> products = findAllProductsByCartId(cartId); | ||
| CartItems cartItems = new CartItems(products); |
There was a problem hiding this comment.
예를 들면 Cart_item 즉, 장바구니 품목을 추가할 때 먼저 DB를 통해 Cart(id, member_id)를 불러옵니다.
그리고 품목을 추가하기 위해서 다시 Cart에서 가져온 cart_id값을 가지고
findAllCartItem(Long cart_id)을 수행시켜서 해당하는 Cart에 포함된 List<Cart_item(id, cart_id, product_id)>를 가져옵니다.
그 후에 다시 가져온 Cart_item을 객체로 만들기 위해서 해당하는 모든 product를 가져온 후에야
객체를 채워주고 Service에서 반환을 해줄 수 있었습니다.
- 카트를 가져올 때, Member, Cart, Product 테이블을 모두 join해서 한 번의 쿼리로 받아올 수 있도록 해보면 어떨까요??
반대로, 현재처럼 테이블을 분리한 상태로 여러번의 쿼리로 호출한다면 개인적으로 크게 이상하다고 생각되지는 않는데요...! ㅎㅎㅎ
- 다만, 이러한 처리를 CartRepository에서 하는게 맞는지에 대해서 고민해보면 어떨까요??? 아래처럼 CartItems가 id를 통해서 간접 참조하는 것과 지금처럼 직접 객체를 가져오는 것과 엮어서 지금 여기서 고민해보면 될 것 같아요 ㅎㅎㅎ
private final List<Long> cartItemsIds;
There was a problem hiding this comment.
시카 리뷰 확인 했습니다~
좋은 고민할 수 있게 해주셔서 감사합니다 :)
2. 다만, 이러한 처리를 CartRepository에서 하는게 맞는지에 대해서 고민해보면 어떨까요??? 아래처럼 CartItems가 id를 통해서 간접 참조하는 것과 지금처럼 직접 객체를 가져오는 것과 엮어서 지금 여기서 고민해보면 될 것 같아요 ㅎㅎㅎ
private final List<Long> cartItemsIds;
기존에 Cart(id, product_id, member_id)로 테이블 및 같은 형식으로 자바 객체를 만들었을 때와 다르게지금 Cart(id, member_id)와 더불어 추가 테이블 cart_item(id, cart_id, product_id)로 만들고,도메인 구조 Cart(Long id, Member member, CartItems cartItems) , CartItems(List<Product> cartItems)를 두었을 때제가 느낀 차이점은후자의 경우로 처리한 경우 비즈니스 로직을 처리할 때 도메인에서 할 수 있는 점이 많았습니다. (ex. cart_item 추가를 할 때 중복을 체킹) 그래서 DB로 들어가기 전에 로직을 수행할 수 있어서 더욱 안정성 있다고 생각했습니다.
- 카트를 가져올 때, Member, Cart, Product 테이블을 모두 join해서 한 번의 쿼리로 받아올 수 있도록 해보면 어떨까요??
여기서, 모두 한번의 SQL을 통해서 받아오고 싶지만, 현재 테이블 구조, 도메인 구조상 두 번의 SQL을 날려야 될 것 같습니다. (db에서 가져온 데이터를 이용해 객체로 만들 때..!)
There was a problem hiding this comment.
제이가 지금까지 한 고민들 모두(대표적으로 패러다임의 불일치) 다른 선배 개발자들도 했던 고민들인데요...! 🤔ㅎㅎㅎ
지금은 직접 Dao로 구현했지만,
이후 미션에서 Spring data를 학습할 때 위 고민들을 어떻게 해결했는지 혹은 더 나은 방법은 없는지 고민해보면 좋을 것 같아요 😄
| public Cart findCartByMember(final Member member) { | ||
| String sql = "SELECT id FROM cart WHERE member_id = ?"; | ||
| Long cartId = jdbcTemplate.queryForObject(sql, Long.class, member.getId()); | ||
|
|
||
| List<Product> products = findAllProductsByCartId(cartId); | ||
| CartItems cartItems = new CartItems(products); |
There was a problem hiding this comment.
제이가 지금까지 한 고민들 모두(대표적으로 패러다임의 불일치) 다른 선배 개발자들도 했던 고민들인데요...! 🤔ㅎㅎㅎ
지금은 직접 Dao로 구현했지만,
이후 미션에서 Spring data를 학습할 때 위 고민들을 어떻게 해결했는지 혹은 더 나은 방법은 없는지 고민해보면 좋을 것 같아요 😄
시카, 안녕하세요? 리뷰이 제이입니다!
이번 미션을 진행하면서 다음과 같은 고민을 겪었습니다. (step1 포함)
도메인 사용에 대해
먼저 Product, Member, Cart 객체를 도메인으로 만들었습니다.
이번 미션에서 사실 "도메인이 필요할까?"라는 생각을 많이 했습니다. 데이터베이스에 저장되는 구조와 같다보니 이런 고민이 들었던 것 같습니다.
그럼에도 도메인으로 구현한 이유는 생성 혹은 수정을 할 때, 객체들의 필드를 각각 원시값 포장을 하면서 검증하기 위한 목적도 있었고
업데이트같은 로직 또한 쿼리로 바로 바꾸는 것이 아닌, 한 번 조회를 하면서 값이 있는지 없는지 확인을 한 후에, 값이 없다면 예외를 보내고 있다면 도메인 메서드를 통하여 수정을 함과 동시에 예외처리를 하고 저장을 하는 방식으로 구현하고 싶었습니다.
예외 같은 부분은 사실 컨트롤러에서 받아오는 사용자의 입력은
@Valid로 해결할 수 있었지만, 어떠한 이유로 사용자의 입력이 바뀌게 된다면 데이터베이스에 잘못된 값이 들어갈 수 있다고 생각했기 때문에, 도메인에서 한 번 더 검증을 진행했습니다.시카는 제가 생각한 것에 대해서 어떻게 생각하시나요??
CartService에서 MemberService + ProductService 사용하는 것에 대해서
장바구니 API를 만들면서 CartService를 사용했습니다.
CartService에서는 다음과 같은 순서로 장바구니에 물품을 저장합니다.먼저
Member를 데이터베이스에서 찾아오는 행위는MemberService에서 진행하고,Product를 데이터베이스에서 찾아오는 것 또한ProductService에서 진행합니다.CartService에서는 이 두 가지 행위가 필요해서 선택을 해야했습니다.MemberRepository+ProductRepository를 둔다.MemberService+ProductService를 둔다.고민 끝에 저는 Repository를 통해서 CartService에서 직접 객체를 가져오는 것이 아닌
다른 Service를 통해서 객체를 가져오는 방식을 택했습니다.그 이유는
Repository를 통해 직접 가져오는방식을 사용하면 서비스가 커지고, 동일한 객체 호출의 반복이 높아진다면 같은 코드를 다른 Service에서 매번 다 작성해주는데, 그렇게 되면 가져오는 로직이 바뀐다면 모두 다 바꿔야한다고 생각했습니다.물론 Repository를 통해서 객체를 가져온다면, 조금 더 유연하게 사용할 수 있는 장점은 있는 것 같습니다. (ex. Optional로 서비스마다 다른 예외를 보내주는 것)
시카는 Service에서 다른 Service를 의존하는 것에 대해서 어떻게 생각하시나요?
따끔한 리뷰 부탁드립니다!
주말인데 감사합니다 :)