leesin1040 / ts_shinee_ticket

0 stars 0 forks source link

코드리뷰 #6

Open BumjuneKim opened 8 months ago

BumjuneKim commented 8 months ago
  1. Layered Architecture에서 배웠듯이 Controller, Service, Repository로 분리하여 Controller는 클라이언트와의 소통 창구, Service는 비즈니스 로직 구현체, Repository는 DB와의 소통창구가 됩니다. 지금과 같이 UserService에서 User Entity 타입을 받아 userRepository를 주입하면 레파지토리를 사용할수 있지만 어떻게 보면 DB와 소통하는 코드가 service에 같이 존재하게 됩니다. 이런 경우 Custom Repository를 만들어보는것은 어떨까요? user 테이블과 소통할수 있는 UserRepository 클래스를 하나 만들어 보는것입니다. 참고 링크를 하나 드릴테니 살펴보시면 도움이 될거에요 https://github.com/leesin1040/ts_shinee_ticket/blob/b1ae0c95b435f9c624b67f25122f26beed4f5c7c/src/user/user.service.ts#L17

[참고링크] https://hou27.tistory.com/entry/TypeORM-Custom-Repository-%EA%B0%9C%EC%84%A0%EC%95%88

  1. ERD가 한눈에 들어오지 않아서 읽기가 좀 힘들지만 일단 review와 user의 관계가 역전된것이 아닌가 싶습니다. 까마귀 발쪽이 user가 아닌 review쪽에 가있어야 할듯한데요. 말로 풀어서 설명하면 일반적으로 한 유저가 여러개의 후기를 남길수 있겠지만 하나의 리뷰를 여러명의 유저가 남길수는 없기에 관계 설정이 반대로 되어 있는듯합니다. 이부분 확인좀 부탁드릴게요

    Screenshot 2024-01-05 at 10 16 17 PM
  2. 이런경우 find보다 findOne을 쓰는게 좋은 것 같아요 https://github.com/leesin1040/ts_shinee_ticket/blob/b1ae0c95b435f9c624b67f25122f26beed4f5c7c/src/show/show.service.ts#L86-L90

  3. 둘다 User entity의 userId로 number 타입이기 때문에 + 연산자는 없어도 될것 같아요. 삼중연산자 비교를 하기 위해 하신듯한데 이중연산자(==)과 삼중연산자(===)의 차이에 대해서도 한번 알아보시면 좋겠습니다. https://github.com/leesin1040/ts_shinee_ticket/blob/b1ae0c95b435f9c624b67f25122f26beed4f5c7c/src/show/show.service.ts#L95-L97

  4. 이미 예약된 seat인지 체크하는것 같은데 book_id가 number형태여서 typeof Number로 비교하신듯합니다. 하지만 일반적으로 number 를 체크하기 위한 방법으론 부적절합니다. 게다가 이 조건은 모두 결과가 false로 나올것입니다. 왜냐하면 typeof Number의 결과는 'function' 으로 나오기 때문입니다. 그 보단 lodash를 사용하시기 때문에 isNumber함수를 사용하거나 typeof findSeat.book === 'number' 로 비교하는게 맞아보입니다. https://github.com/leesin1040/ts_shinee_ticket/blob/b1ae0c95b435f9c624b67f25122f26beed4f5c7c/src/book/book.service.ts#L37-L39

[총평] 스웨거 적용이나 csv 파일업로드등 추가적인 요구사항도 도전해보신것 같습니다. 잘하셨습니다. nestJS 사용해보셔서 아시겠지만 형식이 고정되어 있는 프레임워크라 많이 사용하시다보면 패턴이 익을거에요. 금방 적응해서 실력업 하시리라 생각합니다. 다만 JS/TS 문법이 아직 어색한 부분이 있습니다. 코멘트 드린 부분들 다시한번 체크 해주시길 바랄게요 고생하셨습니다.

leesin1040 commented 8 months ago

찢었다! 섬세하게 피드백 해주셔서 감사합니다!!