Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2단계 - 장바구니 기능] 성하(김성훈) 미션 제출합니다. #291

Merged
merged 43 commits into from
May 10, 2023

Conversation

sh111-coder
Copy link

@sh111-coder sh111-coder commented May 3, 2023

안녕하세요 터틀!! 성하입니다! 😃

이번 미션은 간단한 것 같았는데도 질문할 거리가 많이 생긴 것 같아요 ㅠ_ㅠ 😭

리팩토링 할 것이 조금 있는데, 빠른 피드백을 받아보고 싶어서 일단 빠르게 제출하겠습니다!! 😃

1단계와 마찬가지로, PR에는 선택 분기를 적어보고 해당 선택을 한 이유를 나열하겠습니다!

선택 이유? 가 괜찮은지 판단해주시면 감사하겠습니다 !!! 🙇🏻‍♂️

나머지 질문들은 코드단에 달도록 하겠습니다!! 😃


🎯 선택 분기점

✅ 1. 패키지 구조 계층형 VS 도메인형 : 계층형 선택

1-1. 계층형

controllerproductmembercart

serviceproductmembercart

...

exceptionproductmembercart

dtoproductmembercart
  • 장점

    • 프로젝트 전반적인 이해도가 낮아도, 패키지 구조만 보고 전체적인 구조를 파악할 수 있다.
    • 애플리케이션의 흐름 따라가기가 좋다.
      • 상품 등록 상황
        • 애플리케이션 흐름 = 아키텍쳐 구조이므로, controller → service → dao 패키지대로 내려가면 흐름을 자연스럽게 파악할 수 있다.
    • 계층별 응집도가 높아진다.
  • 단점

    • 도메인별 응집도가 낮다.
      • Product 도메인의 흐름만 보고 싶을 때, 모든 아키텍쳐 패키지를 봐야한다.
    • 도메인이 늘어나면 패키지 안에 클래스들이 많아질 수 있다.

1-2. 도메인형

productcontrollerservicedaodto

membercontrollerservicedaodto
  • 장점

    • 도메인별 응집도가 높다.
      • Product 도메인의 흐름만 보고 싶을 때, product 패키지만 보면 된다.
  • 단점

    • 도메인으로 나눠서, 해당 클래스가 어떤 도메인 패키지에 들어갈지 애매한 경우가 있다. (개발자마다 생각이 다른 경우가 있다!)
      • 저는 이번에 'HomeController'가 존재하는데, 이와 같은 클래스는 도메인 구조로 했을 때 어디로 들어갈지 애매했습니다!

계층형을 선택한 이유

Layered Architecture를 사용하는 입장에서 계층별로 나뉘어진게 훨씬 가독성이 좋다고 생각했고,
애플리케이션 전반의 흐름을 읽기가 쉽다고 생각했습니다!
또, 현재는 도메인이 적기 때문에 도메인이 흩어져 있더라도 계층별로 하는 것이 더 좋다고 생각해서 계층형 구조를 선택했습니다!


2. ✅ AdminController 네이밍 : ProductController로 변경

AdminController에 대한 고민을 했던 부분은 다음과 같았습니다.
admin 페이지와 관련된 API는 Product 관련 API입니다.

패키지 구조와 AdminController 네이밍만 봤을 때, 해당 Controller가
Product API와 관련된 Controller임을 알기가 힘들 것 같았습니다.

controller
	⎿ AdminController

그래서 다음과 같은 해결 방안을 생각해봤습니다.

1. AdminProductController로 네이밍 변경
2. ProductController로 네이밍 변경
3. controller 패키지 안에 admin 패키지를 나누고 ProductController로 네이밍 변경

1번은 admin 페이지에 추가적인 기능이 생기면, 계속 AdminXxxController로 생성되야하는 것이 좋지 않다고 생각해서 선택하지 않았습니다.

2번은 Product에 관한 API Controller 라는 것이 명확해졌지만, product를 admin이 관리하는 느낌이 사라져서 좋지 않다고 생각했습니다.

그래서 최종적으로는 3번을 택해서 다음과 같은 패키지 구조를 가지게 했습니다!

controller
	⎿ admin
             ⎿ ProductController

한 가지 걸리는 점은 패키지 구조를 계층형으로 나눴는데, controller 패키지 안에 계층 느낌이 아닌

admin 패키지가 있어도 되는지 조금 마음에 걸립니다!

괜찮은 걸까요?


  1. 테이블 설계 시 CART 테이블을 만들어야 할까?

처음에는 DB 설계 시에는 다음과 같이 설계했습니다.
image

MEMBER와 1:1 관계의 CART 테이블을 생성하고, MEMBER가 FK로 CART의 PK를 가�졌습니다.

또, CART와 PRODUCT는 M:N이기 때문에 중간 테이블을 둬서 1:N, 1:N으로 분리했습니다!

그런데, CART 테이블에 PK만 존재하게 되는데 존재해야할까? 라는 생각이 들었습니다!

이럴 경우 불필요한 JOIN이 일어나서 성능이 더 느려질 것 같았습니다!

그래서 기존 CART 테이블을 삭제하게 되었습니다.

이렇게 되면, MEMBER와 PRODUCT가 M:N이 되므로 중간 테이블을 둬서 1:N, 1:N으로 분리하�였고,

member_id와 product_id를 FK로 가지고 있는 중간 테이블이 CART의 역할을 할 수 있다고 생각하게 되었습니다!

그래서 중간 테이블의 네이밍을 CART로 하게 되었습니다!!

다음은 최종 DB 설계 ERD입니다!
image


✅ 4. 인증 정보로 사용자 존재 여부 확인 기능 구현 방법

인증 정보로 사용자 존재 여부 확인 기능을 구현할 때 2가지로 구현 방법을 생각했었습니다!

  1. ‘사용자’ 존재 여부 확인이니까 MemberSerivce에서 MemberDao를 가지고 구현한다.
  2. ‘장바구니’의 모든 기능이 사용자 존재 여부 확인 기능을 사용하니까 사용자 존재 여부 확인 기능을 장바구니의 비즈니스 로직으로 보고 CartService에서 MemberDao를 가지고 구현한다.

저는 2번 방법을 사용했습니다!
1번 방법으로 하지 않은 이유는
CartController에서 사용자 존재 여부를 확인하기 위해 MemberService를 의존하거나,
CartService에서 MemberService를 의존해야� 했었습니다.
이때 장바구니에 대한 컨트롤러가 Member의 비즈니스 로직 계층인 Service에 의존하는 것은 이상하다고 생각했고, 마찬가지로 장바구니 Service가 Member 비즈니스 로직을 의존하는 것도 이상하다고 생각했습니다.

DAO는 DB단에 접근하는 비즈니스 로직과는 관련없는 계층이어서, CartService에서 다른 도메인의 Dao를 가져도 된다고 생각해서 2번으로 선택했습니다!

MEMBER에서 cart_id 삭제,
CART_PRODUCT -> CART로 테이블명 변경
CART에서 member_id, product_id FK로 가지도록 변경
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
return simpleJdbcInsert.executeAndReturnKey(params).longValue();
}

public List<ProductEntity> selectAllProductByMemberId(Long memberId) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q : JOIN을 할 때 JOIN한 결과를 어떤 객체로 반환해야할까?

현재 CartDao에서 PRODUCT 테이블과 JOIN해서 장바구니에 담긴 상품들을 가져오고 있습니다!

이때 필요한 정보들은 PRODUCT 테이블에 있는 정보들이기 때문에 ProductEntity를 반환 객체로 사용했는데,

CartDao에서 CartEntity가 아닌 ProductEntity를 쓰는 것이 조금 찝찝하더라구요..

그렇다고 CartEntity를 반환하자니 JOIN을 쓰는 의미가 없어서 고민이 됩니다!!

ProductEntity를 반환해도 괜찮을까요??

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

답변드리기 전에 Product 중심으로 조회가 이루어지는데 CartDao에 구성해주신 이유는 무엇인가요?

Copy link
Author

@sh111-coder sh111-coder May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Join을 할 때 CART 테이블의 product_id FK로 PRODUCT의
단순하게 Join을 하는 주체를 PRODUCT 테이블 보다는 CART 테이블로 봐서
CART 테이블과 1:1로 대응되는 CartDao에서 처리한 것 같아요!

그리고 처음에 ProductDao에 Join해서 Product를 조회하는 것은
장바구니에 담을 상품을 처리하는 것이라서 의미적으로 ProductDao 보다는 CartDao에서 처리하는게 맞다고 생각했습니다!

그런데 지금 생각해보니 Dao 계층은 DB 로직만 처리하는 계층으로, '장바구니에 담을 상품을 처리하는 것'이라는
비즈니스 로직? 부분을 모르고 단순히 'CART 테이블과 JOIN하여 조건에 맞는 상품을 가져오는 것'이라는 의미로 보면
ProductDao에서 JOIN을 해도 괜찮다는 생각이 들었습니다!

하지만 memberId를 파라미터로 받아서 memberId가 같은 행만 가져와서 Join하는데
ProductDao에서 memberId를 사용하는 것이 의미적으로 이상하다고 생각했습니다!

그래서 CartDao에서 조회를 하는 것은 그대로 두려고 합니다!


추가적으로 리팩토링하고 싶은 부분은 현재 장바구니의 상품 삭제를 PK가 아닌 FK 2개(memberId, productId)를 받아서
진행하고 있습니다.

이는 조회 시에 ProductId를 반환해서 삭제도 ProductId를 이용하게 하기 때문이었습니다!

그래서 장바구니의 상품 삭제를 PK로 변경하는 것이 맞다고 생각이 들어서
조회 시에 ProductEntity가 아닌 CartId를 포함한 Entity를 새로 생성하고 싶은데,
조인할 때마다 이렇게 Entity를 새로 생성해야 하는지 궁금합니다 ㅠㅠ 😭

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고민 해결 방향이 좋네요 👍🏻 말씀해주신 이유도 좋은데 이런 경우가 반복된다면 모델링을 다시해봐야하는 신호일 수 있다는 점도 생각해보시면 좋을것같아요.


Entity라서 고민이 되는것같은데 객체지향이 아닌 관계형 데이터베이스에서 쿼리하는 경우로 생각해볼까요?

조인하는 이유는 다른 테이블에서 필요한 컬럼이 생긴 경우일텐데, 그렇다면 SELECT 항목에 포함시키게 될거에요. 해당 데이터를 객체지향까지 가져오는것이 Entity를 추가하고 모델링을 하는 과정이라서 필요한 과정 중 하나입니다.

대신 매번 모델을 만들면 번거롭기때문에 성능에 큰 문제가 없다면 필요한 필드를 골라 모델을 만드는것보단 테이블에 있는 모든 필드를 갖고있는 모델(일반적으로 Entity)을 재사용하기도 한답니다.

Copy link
Member

@begaonnuri begaonnuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 성하!
이번에도 질문이 상당하시군요.. 열심히하시는게 느껴져서 저도 덩달아 열심히 리뷰하게되네요🥲
몇가지 코멘트 남겼는데 확인해주세요.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
src/main/java/cart/controller/CartController.java Outdated Show resolved Hide resolved
return ResponseEntity.ok(allProduct);
}

@PostMapping("/cart/{productId}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path Variable은 리소스를 식별하는 용도로 사용하는게 좋다고 생각해요.

지금은 식별자인 아이디 하나만을 입력받아서 헷갈리셨을 수도 있을것 같은데, 만약 상품 아이디(productId)와 수량(quantity)을 정보를 함께 받는다고 가정하고 코멘트로 요청 API 스펙을 구성해보시겠어요? 그걸 보면서 다시 얘기해봐요 😊

Copy link
Author

@sh111-coder sh111-coder May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 URI가 '서버에게 요청할 리소스를 표현한 것'이라고 생각합니다!
그리고 Path Variable은 해당 리소스를 계층으로 표현하는 방식이라고 생각합니다!

장바구니에 상품을 담을 때는 아직 상품이 없으므로 제 URI에서 cart 아래 계층이 있다는 것이 어색한 것 같습니다!!

그래서 POST '/cart'로 하고, Query String을 사용해서 상품 정보를 넘겨 주는 게 좋다고 생각이 들었어요!

상품 아이디, 수량을 함께 받는다면 'POST /cart?productId=1&quantity=10'이렇게 될 것 같은데, 괜찮을까요?!

Query String이 적절한지는 모르겠지만, 저는 Query String이 주소창에 정보(productId, quantity)가
직접 노출되기 때문에 조금 별로라는 생각이 들었어요!
quantity는 상관이 없을 수 있지만, productId는 식별자라서 노출되는 것이 찝찝한데 사용해도 괜찮은 걸까요?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다! 수량같은 필드가 있는 경우 POST /cart?productId=1&quantity=10과 같은 형태로 확장되기도하고 성하가 말해준 이유까지 더해서 POST /cart 정도면 어떨까 싶네요.

그리고 일반적으로 POST 요청에선 Query String을 사용하지 않고 body를 사용합니다. body로도 해결할 수 있으니 body를 쓰는게 어떨까요? Query String을 사용하려고 하신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개념적으로 PathVariable과 Query String이 묶여서 Body에 담는 건 생각을 못했던 것 같아요!

제가 말한 노출의 이유때문에 Query String 대신 Body를 사용하는 걸까요?

body를 사용해서 리팩토링하도록 하겠습니다!!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개념적으로 PathVariable과 Query String이 묶여서 Body에 담는 건 생각을 못했던 것 같아요!

그래서 일반적으로 POST 요청에 PathVariable도 잘 안쓰이긴 합니다.

Body를 사용하는 이유는 다양한데 말씀해주신 보안적인 이유도 있고, 게시판을 작성한다고 할때 게시글의 내용이 url에 포함된다고 하면 무의미하게 길어지지 않을까요? 그리고 REST API 스펙 자체가 그렇기도 합니다.

src/main/resources/templates/cart.html Show resolved Hide resolved
@begaonnuri
Copy link
Member

리뷰하다보니 코멘트가 꽤 많아져서 PR에 질문주신 부분은 다음 리뷰때 답변드리겠습니다 🙂

@sh111-coder
Copy link
Author

sh111-coder commented May 5, 2023

안녕하세요 터틀!!

궁금했던 부분이 많았어서 질문이 좀 많고 길어졌던 것 같아요 ㅠㅠ 😭

친절하게 답변해주셔서 감사합니다!! 많은 도움이 되었어요! 👍

리팩토링한 부분은 별로 없지만, 질문에 답변을 달면서 많은 지식을 얻어간 것 같아요!

답변해주신 것 중에 이해가 안됐던 부분이나 질문이 생긴 부분은 코멘트로 질문 남겨놨어요!

Join 관련해서 어느 Dao에서 Join할지, 반환은 어떻게 할지 처음 접한 부분이 많아서 어려운 것 같아요 ㅠㅠ 😭

@begaonnuri
Copy link
Member

저는 도메인으로 패키지를 구분하는것을 선호하긴 합니다. 지금은 도메인의 규모가 크지 않지만 점점 도메인을 중심으로 확장되는데 이때 계층형으로 패키지가 구성되어있으면 flat하게 펼쳐져서 패키지가 커지는 경향이 있습니다.

그렇게되면 패키지별로 접근 제어나 변경 사항의 경계가 모호해지는 단점이 있습니다. 접근 제어가 뚜렷해지지 않으면 무분별한 참조가 많아지고 코드를 작성하거나 리뷰할때 패키지 import 부분을 자세히 보지 않으면 찾아내기 어렵습니다. 그리고 규모가 커져서 리팩토링할때 많아진 참조를 끊어내기도 어렵습니다.

하지만 이런건 도메인 규모가 큰 경우의 얘기고 지금은 그런 단점을 못느끼셨으니 계층도 좋고 성하가 말해준 장점도 있습니다. 저라도 지금 규모에선 계층형으로 할수도 있을것같네요.

패키지나 아키텍처에 대한 얘기는 다루는 책도 많고 할 얘기가 많지만 상대적으로 어려운 주제이니 천천히 고민해보셔도 좋을것같네요.


패키지로 어드민을 구분하는건 좋아보여요. 스프링에서 설정을 하더라도 클래스로 나열하면 너무 많아져서 패키지 기반 설정하는경우가 많아서 어드민 패키지만 인증 방식을 변경하거나 할때도 패키지 설정이 유용할거에요.


잘 고민해주신것같아요. 최종 ERD와 같은 형태가 더 좋아보이네요!


서비스가 점점 많아질텐데 전부 MemberService나 MemberDao를 갖게되지 않을까요? 그러면 Controller로 오기전에 @AuthMember로 오는 회원은 모든 인증을 마친상태라고 보장하게되면 더 확장성있지 않을까요?

Copy link
Member

@begaonnuri begaonnuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 고민해주셨네요 성하 👍🏻
리뷰 드리기엔 충분히 잘 구현해주셨고, 몇가지 코멘트 남겼으니 답변만 다시 한번 확인해주세요.

@sh111-coder
Copy link
Author

sh111-coder commented May 8, 2023

안녕하세요 터틀!!

JoinEntity 생성 & 상품 POST URI 수정 & ExceptionHandler 부분 리팩토링을 진행했습니다!!

서비스가 점점 많아질텐데 전부 MemberService나 MemberDao를 갖게되지 않을까요? 그러면 Controller로 오기전에 @AuthMember로 오는 회원은 모든 인증을 마친상태라고 보장하게되면 더 확장성있지 않을까요?

이 부분은 생각 못해봤었는데, AuthInterceptor 부분에서 MemberDao를 가지고 검증을 진행하면
이후에 서비스가 확장되어도 공통적으로 처리가 될 것 같다고 생각했어요!!

이 부분은 6시 전까지 시간이 부족할 것 같아서 먼저 리뷰 요청을 보내고 저녁에 리팩토링을 해보려고 합니다!!

  • 저녁 8시 리팩토링 완료했습니다!

Copy link
Member

@begaonnuri begaonnuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 잘 반영해주셨네요 성하!
몇가지 남긴 코멘트가 있는데 전체적으로 코멘트 한번 확인해주세요.

이번 미션은 여기서 머지하겠습니다.
확인이 늦어져서 다시 한번 죄송하고, 궁금하신점 있다면 편하게 DM주세요 😊

@begaonnuri begaonnuri merged commit 0b71890 into woowacourse:sh111-coder May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants