부산대 BE_정지민 4주차 과제(3단계)

[stopmin]

이번주 과제는 전반적으로 여태 마음은 불편했지만 배설급으로 싸낸 못생긴 코드들을 리팩토링 하는데 시간이 더 걸린 것 같습니다! 지난주부터 느꼈는데 구현하다보니 인증/인가 부분이 많이 아쉽더라고요! 그외에도 더 잘할 수 있었는데 다른 프로젝트 시간 이슈로, 불편했지만 고치지 못했던 부분들이 많이 보여서 step1~step3까지 전반적으로 그런 부분들을 다시 바로잡는데 오래 걸린 것 같네요! 솔직히 아직도 조금 마음에 안드는데, 멘토님 리뷰 반영해보면서 조금 더 퀄리티 높게 수정해보도록 하겠습니다!!! 보면 볼수록 더 개선해보고 싶고 바꿔보고 싶은게 백엔드 코드같네요!

바쁘시겠지만,, 선행하는 PR에 코멘트들도 한번 확인해주셨으면 좋겠습니다~~!!

선행 PR

이전 PR이 닫히지 않아 참고하시면 좋을 듯 하여 밑에 링크 달아둡니다~~!! 확인 부탁드립니다 :D 늦어서 미안합니다😢

• step0 부산대 BE_정지민 4주차 과제(0단계) #366
• step1 부산대 BE_정지민 4주차 과제(1단계) #369
• step2 부산대 BE_정지민 4주차 과제(2단계) #374

step3 의 목표는 결제 상황을 대비하자!같네요.
옵션 수량을 감소할 이유가 뭐 위시리스트 삭제.. 그런것도 있겠지만 일단 결제한다고 생각하고 구현했습니다.

구현 사항

• 결제라고 가정했으니 이건 WishList의 도메인이 아니다 싶어 payment도메인을 추가 생성했습니다. (패키지를 새로 만들었습니다.)
• 결제 API를 만들었는데 이전 PR들과 유사하게 AccessToken을 통해 유저 id를 가져오고 해당 위시리스트에 대한 접근 권한이 있는지 validation 하였습니다.
• valid한 경우, stock의 개수를 줄여주었는데, 이때 productItem 삭제, wishList 삭제 (이거 꼭 해야할지 잘 모르겠네요), WishListProduct에서 개수를 가져와 해당 옵션의 개수를 내려주었습니다.(이때 모자라면 재고 없다고 예외 발생)

또한 앞선 pr에서 연관관계 관련돼서 잘못된 부분이 꽤 보여서 이번 PR에서 조금 개선하기도 하였습니다. 따라서 step3 보시고 리뷰 달아주시면 될것 같습니다!

[syndersonlee]

👍 늦게 리뷰 드려서 죄송합니다!

우선 전체적으로 코드나 README.md를 전부 읽어봤습니다. 대부분 목표하신 코드는 잘 구현 되었습니다. 그 부분은 보기도 좋습니다. commonResponse 부분은 제가 쓰는 방식이라는 다르지만, 틀리다보다는 어떤 회사는 그런식으로 작성하는 것도 보았기에 이해할 수 있었습니다.

지민님의 코드에서 가장 개선이 필요한 부분은 대부분의 파라미터를 분해해서 넘기고 있는 점입니다.
예를 들어서 생성자를 기존에는

public Category(String name, String description, String imageUrl, String color) {
        this.name = name;
        this.description = description;
        this.imageUrl = imageUrl;
        this.color = color;
    }

위 처럼 만들었다면, 앞으로는

public Category(CreateCategoryRequest request) {
        this.name = request.name;
        this.description = request.description;
        this.imageUrl = request.imageUrl;
        this.color = request.color;
    }

위 처럼 구현하시길 바랍니다. 대부분에 메소드에서 파라미터를 넘길 때 나눠서 넘기다보니 가시성이 떨어지고 테스트가 어렵습니다 :) 그리고 setter나 getter 대신 필요한 부분을 가져오는 함수를 직접 정의하는 걸로 바꿔가는 게 중요합니다. 필요하다면, 그 방식의 중간 DTO를 생성해서 전달해주는 것도 좋습니다 :)

[stopmin]

지민님의 코드에서 가장 개선이 필요한 부분은 대부분의 파라미터를 분해해서 넘기고 있는 점입니다.
예를 들어서 생성자를 기존에는
public Category(String name, String description, String imageUrl, String color) {
this.name = name;
this.description = description;
this.imageUrl = imageUrl;
this.color = color;
}

와!! 이 부분 이번에 고쳤다고 생각했는데 이번에 짚어주셔서 바로 이해했습니다!!! 와우.... 그런식으로 짠다면 조금 더 가시성이 높아지겠네요!!!!! 왜 이 생각은 못했을까요 !!! 좋은 피드백 감사합니다~~~

commonResponse는 지난번에도 말씀해주셨던 것으로 기억합니다! 그런데 말씀하신대로 회사에서 이런 방식으로 사용했었는데 편했기 때문에 사용중인데 틀리진 않았지만 범용적이지 않다면 다음 미션 때 부터는 사용하지 않도록 하겠습니다!

생성자 부분 수정 일괄로 하고 슬랙 DM 드리겠습니다~~!!

좋은 피드백 감사합니다😄

[stopmin]

setter나 getter 대신 필요한 부분을 가져오는 함수를 직접 정의하는 걸로 바꿔가는 게 중요합니다. 필요하다면, 그 방식의 중간 DTO를 생성해서 전달해주는 것도 좋습니다 :)

이 부분에 대해서는 제가 코틀린만 쓰다 와서 그런지 아직은 무감각 한 것 같아, 감을 많이 익혀봐야 할 것 같습니다~~!! 책/강의/구글 통해서 최대한 매꾸어보도록 하겠습니다!!! 다음에 리뷰에서 또 아쉬운 포인트, 여기서 setter, getter 쓰면 안되겠다 싶은 부분 있으면 지적 부탁드립니다😄
지금 귀찮아서 다른 프로젝트에서는 롬북... 테러 하고 있습니다😓 반성해야겠습니다..