Closed whdqhdqkr closed 2 years ago
@ken1336 요거 코드리뷰 챙겨주세요
This PR has not had activity in 30 days. Closing due to staleness.
@wnsgml972 @ken1336
조금 다르게 생각하는 부분이 있어서 검토 요청드립니다.
CoffeeInsertRequest의 경우 Insert를 위한 DTO라고 생각이 들었습니다. 만약 Insert를 하지 않고 이미 Insert된 값을 참조하는 상황에서도 CoffeeInsertRequest DTO로 처리하는 게 맞는지 검토를 부탁드리겠습니다.
저는 DTO라는 관점에서는 괜찮을지라도 Insert 요청 전용의 DTO같아서 CoffeeDTO에 id를 넣는게 어떤가 싶었고 CoffeeMapper를 수정하게 되었습니다.
제가 직접 개발한 영역이 아닌만큼 이야기 후에 충분히 다시 수정할 것을 염두하며 제시해봅니다. 두분의 의견을 기다리겠습니다.
직접 구현하신 @ken1336 님이 대답해주시면 좋을 것 같네요
@whdqhdqkr 답변이 많이 늦어져서 죄송합니다. 최근 회사일이 많아져서 주말밖에는 작업할 시간이 없네요. 조금 양해부탁드립니다.
수정하신 부분은 모두 확인해 봤고, 의견주신 CoffeeInsertRequest대신 CoffeeDto에 id를 넣고 사용하는 방법에 대해서도 이견이 없습니다. 그런데 한 가지 추가로 궁금한점이 생겼습니다. 조금 더 근본적으로, '작성하신 OrderingDetailRequest에 coffee, noncoffee, bread, bean의 dto가 굳이 들어가야 하는가' 입니다.
Ordering 도메인의 기능은 사용자가 주문을 했을때 주문 상세 옵션과 메뉴(product 도메인)를 연결해 주는 것으로 정의할 수 있을것 같습니다.
그런데 OrderingDetailRequest에 product 도메인의 정보를 포함한다는 것은 OrderingDetailRequest에 필요 이상의 정보를 제공한다는 생각이 들었습니다. 예를들어 '커피를 주문'하는 과정에서 request에 포함되는 coffeeDto의 List
말로만 하긴 좀 그래서 OrderingDetailRequest를 아래와 같이 변경해 보았습니다. (feature/min-order-modeling)
data class OrderingDetailRequest( val coffeeName: String?, val nonCoffeeName: String?, val breadName: String?, val beanName: String?, val drinkOptionRequest: DrinkOptionRequest?, val breadOptionRequest: BreadOptionRequest?, val beanOptionRequest: BeanOptionRequest?, )
클라이언트에서 request를 보낼 때 OrderingDetailRequest에 product의 dto 대신 product의 id를 넘겨주고, 서버의 service layer에서 save하기 전 id에 맞는 product entity를 찾아 OrderingDetail에 추가하도록 했습니다. 실제로 OrderingControllerIT를 실행해보면 save하기 전에 product entity를 찾는 쿼리가 한번 더 나가기는 합니다.
말씀해주신 내용을 보고 이것저것 생각해보다가 여기까지 왔습니다. 같이 의견 나눠보면 좋을것 같습니다!
@ken1336 길고 매우 값진 피드백 남겨주셔서 감사합니다. 말씀하신 것처럼 DTO가 필요하진 않을 수 있다고 생각합니다! name을 통해 다시 조회를 한다하더라도 접근 가능한 최소한의 정보로 가지고 있는거라면 이 문제를 해결해나가는 것도 좋다고 생각합니다.
제안해주신 아이디어 빠른 시일 내에 반영해볼게요!
저도 퇴근 후엔 할 힘이 없고 주말에 주로 진행합니다 ㅎㅎ 늦고 안늦고는 중요하지 않은거 같습니다! 제 코드를 이해해보셨고 거기서 피드백과 아이디어까지 주셔서 정말 감사합니다. 💯