Step2#45
Conversation
qhals321
left a comment
qhals321
left a comment
There was a problem hiding this comment.
안녕하세요 현준님! 이번에도 가독성 좋은 코드로 깔끔하게 구현해주셔서 쉽게쉽게 읽어 나갔네요!
특히, Item 을 만들어주셔서 Product 와 Member 를 연결해주신 설계 너무 좋았습니다~! 💯
추가적으로 이번 미션에서 충분히 이야기가 되었으면 좋을 부분이 있습니다!
- dto (request, response) 는 현준님에게 어떤 의미일까요?! 어떤 repository 에서는 도메인 모델을 파라미터로 받으시고 어떤 repository 에서는 request 를 직접적으로 파라미터로 받으시는 부분이 있으셔서 어떤 기준으로 진행하시는지 궁금했습니다! (한 번 읽어보셔도 좋을 글 남깁니다~. https://tecoble.techcourse.co.kr/post/2021-04-25-dto-layer-scope/)
- 스프링 기능 활용 수업인 만큼 Interceptor 와 ArgumentResolver 스프링 기능을 활용해 유저에 대한 인증을 진행해보시는 것도 좋을 것 같아요!
또한 코드 관련해서 커멘트도 남겼습니다~. 확인 부탁드려요! 🙇 이번 미션도 파이팅입니다~. 🔥
| return new MemberDetails(email, password); | ||
| } | ||
|
|
||
| throw new MemberNotFoundException(ErrorCode.NOT_FOUND_MEMBER); |
There was a problem hiding this comment.
404 에러로 나오게 되네요!!
회원이 존재하지 않을 경우 404 인 케이스도 있지만 해당 부분 같은경우는 404 보다는 401 이나 403 같은 권한 관련 에러가 더 맞을 것 같은데 어떻게 생각하시나요!
| public List<ItemResponse> getItemsByMemberId(@RequestHeader String authorization) { | ||
| Member member = authService.validate(authorization); |
There was a problem hiding this comment.
이렇게 보내게 되면 서비스 로직에서 http 스펙 (Authorization 헤더 스펙) 과 커플링이 일어날 것 같아요!
http 스펙 관련은 컨트롤러 레이어에서 끊어내는 건 어떨까요?!
추가적으로 ArgumentResolver 와 같은 스프링 기능도 한 번 이용해보셔도 좋을 것 같아요!
|
|
||
| public List<ItemResponse> findAllByMemberId(Long id) { | ||
| return itemDao.findAllByMemberId(id).stream().map(item -> { | ||
| Product product = productDao.findById(item.getProductId()).orElseThrow(() -> new ProductNotFoundException(ErrorCode.NOT_FOUND_PRODUCT)); |
There was a problem hiding this comment.
대부분 404 에러로 나가게 되는데 현준님이 생각하시는 404 에러에 대한 정의가 궁금해요!
Item 을 찾았지만 Item 이 없는 경우가 아닌 Product 가 없는 경우도 404 에러는 어색해보여서요!
| public void addCarItem(ItemAddRequest itemAddRequest) { | ||
| Product product = productDao.findById(itemAddRequest.getProductId()).orElseThrow(() -> new ProductNotFoundException(ErrorCode.NOT_FOUND_PRODUCT)); | ||
| itemDao.insert(itemAddRequest.setProductId(product.getId())); | ||
| } |
There was a problem hiding this comment.
트랜잭션을 이용해서 기능에 대한 원자성을 만들어주시는 것도 좋을 것 같아요!
| @Getter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class Item { |
There was a problem hiding this comment.
Item 을 만들어서 회원과 상품이 직접적으로 연결되는 걸 막아주는 설계! 너무 좋네요~ 💯
2 participants
Uh oh!
There was an error while loading. Please reload this page.