# Week5 피어 세션
> 참여자 : J186 정수원

## 활동 목적
- 한 주 되돌아보기
- 배울 만한 점 배우기

## 진행 순서
- 커밋 브리핑
- 코드 리뷰

## 커밋 브리핑 / 시연
> 순서 :

## 코드 리뷰
##### J105 신준수
- (정수원)
- backend/views/error.ejs 에러 뷰를 백엔드에 따로 놓은 이유가 있을까요?
- 모듈 여기저기 done을 인자로 받아서 리턴을 done(null, user) 해주셨는데 이 모듈이 어떻게 생겼는지, 역할은 무엇인지 궁금해요, 찾아보려고 했는데 못찾겠어서 질문합니다!
- 전체적으로 군더더기 없이 깔끔하게 짜신것같은데 제가 패스포트, 깃헙 auth를 쓰지 않아서 아쉽네요ㅠㅠ 썼더라면 참고할 부분이 많았을 것같아요 👍🏻

- (임채욱)
- pm2 config를 이용하여 NODE_ENV를 다르게 지정하는 방법 되게 좋은것 같아요
- backend, frontend 디렉토리 외부에서 eslint와 prettier 빌드를 위한 설정을 하는 방법 좋아보이네요

- (탁성건)
- sequelize 사용 시 Model 간의 관계 설정을 extra-setup.js에 따로 분리를 해서 관계를 파악하기 편했습니다.
- frontend의 .csscomb.json 파일의 용도가 궁금합니다.

##### J171 임채욱
- (정수원)
- back/src/db 디비를 클래스 객체로 만들어서 constructor()에 모듈을 require한 방식 신기하네요! 저는 처음보는 형태인데, 혹시 이 부분 짜는데 참고하신 디자인 패턴이 있을까요?
- db 모듈을 user, transaction 나누지 않고 한 클래스에 넣은 이유가 있을까요?
- 이건 제 사견입니다! db-api.js 랑 user.js 라우트가 같은 역할(api 요청)을 하는 것 같은데 파일 이름을 통일해주면 보기 좋을것같아요! 예를 들어 user-api.js 처럼요 :)
- 길어지는 로직을 user-controller.js 따로 컨트롤러로 뺀 건 좋은 것 같아요 👍🏻

- (신준수)
- 칭찬:
- Prettier 설정이 디테일하게 되어있어서 좋네요.
- isOwnerTransaction 함수로 유저의 권한을 확인해준 부분이 인상 깊었습니다.
- 질문 & 리뷰:
- db.js에서 DAO와 유사한 인터페이스를 제공하고 있는데, ORM을 쓰는 환경에서 또 하나의 인터페이스가 필요하다고 느끼신 포인트가 궁금합니다.
- /pays(GET)와 /pay(POST) 라우팅 주소가 다른 이유가 궁금합니다. (transaction은 모두 동일하네요)
- Backend와 Frontend prettier 세팅을 별도로 가져가시는 이유가 궁금합니다.

- (탁성건)
- sequelize-models에 model들을 정의해두고 db.js에서 model들을 모아 추가적으로 DB 접근에 필요한 함수들을 정의하셨는데 관리의 편의성 측면에서 이렇게 구성하신것인지 궁금합니다.

##### J186 정수원
- (임채욱)
- util 디렉토리를 구성하여 필요한 코드 및 query를 따로 관리하는것이 되게 좋은거 같습니다.

- (신준수)
- 칭찬:
- 회고를 세세하게 적으시고, 또 참고한 자료의 reference까지 적어놓으신 점이 정말 좋네요.
- app.set(JWT_SECRET)으로 처음 express app에 변수를 세팅해서 쓰는 부분이 좋았습니다. 이렇게하면 매번 config나 env를 임포트해줄 필요가 없네요.
- util함수를 여러가지 짜서 사용하시는 부분이 좋았습니다. 혹시 추후에 FE와 공유한다면 어떻게 폴더구조를 잡으실 예정인가요?
- 질문:
- Promise 패턴과 async/await 패턴을 혼용하는 부분이 많은데, 두 패턴을 함께 사용하신 이유가 궁금합니다.(개인적으로는 혼용하면 가독성이 조금 떨어지는 것 같아요)
- Router에서 controller를 따로 리팩토링 해서 빼셔서 같은 폴더에 넣어주신 것 같은데, 이와같이 폴더구조를 잡으신 이유가 궁금합니다.
- 개선사항:
- README.md에 요구사항 페이지 링크가 깨져있어요 ㅠ ㅜ
- verifyJWT 함수와 같은 부분에서 부분적으로 설명하는 주석이 달려있는데, 정작 코드가 간결하고 naming이 잘 되어 있어서 주석이 없어도 될 것 같은 느낌입니다. (최근 클린코드 읽고 있는데, 저자는 코드 자체가 명료하고 주석이 없는 것이 베스트라고 하더라구요)
- util에서 json to array는 Object.values()로 대체 가능할 것 같아요.
- 중간 promise 패턴에서 쓰시는 변수명 'p'는 가독성이 떨어지는 것 같아요.
- user model checkExist 함수에서 resQuery[0][0]과 같이 쓰는 부분은 magic number가 들어가는 것이 아닐까요?[[response]] = resQuery로 써도 좋을 것 같아요.
- express-generator에서 만들어준 user router가 아직 남아있네요.

- (탁성건)
- transController.js에 사용하신 문법이 객체 리터럴이라고 하는 게 맞나요?? 코드가 훨씬 깔끔해지는 느낌이네요

##### J211 탁성건
- (신준수)
- 칭찬:
- README에 요구사항 정리해놓으신 부분이 좋았습니다. 맨 아래쪽에 직접 나누신 요구사항은 체크가 되어있지 않던데 Frontend에만 해당하는 부분들인가요?
- configs를 정리한 부분이 좋았습니다. 저는 .env에 다 모아서 관리하는데 DB, jwt 이런식으로 도메인별로 따로 하니까 더 보기 좋은 것 같아요.
- POST 요청 시 데이터가 겹치면 409 코드를 쓰셨는데, status code로 semantic을 전달하시기 위해 고민하신 노력이 보여서 정말 좋았습니다.
- 전반적으로 코드가 간결해서 보는 입장에서 읽기가 편했던 것 같아요!
- 질문:
- jwt verify 함수에서 user를 sequelize에서 뽑은 그대로 넘기시지 않고, 별도의 object로 변환하신 이유가 궁금합니다.
- 로그인 시에는 passport를 사용하시지 않고, api 토큰 인증에만 passport를 사용하신 것으로 보입니다. 이렇게 passport의 사용처를 나누신 이유가 따로 있는지 궁금합니다.
- Payment 테이블에서 row를 테이블에서 삭제하는 대신 isDeleted Column을 세팅하신 이유가 궁금합니다. 결제수단이 삭제되어도 transaction 테이블에서 foreign key 참조가 깨지지 않기 위함인가요?

- (정수원)
- .gitignore 에 뭔가 내용이 많네요 😲
- 아 라우트 폴더구조 깔끔하고 좋습니다 👍🏻 따라가기 편하네요!
- router -> service -> model 흐름을 깔끔하게 잘 잡으신것같아요
- 씨퀄라이즈 쓰신 다른분들도 그렇겠지만, db읽기가 확실히 간결하군요. 깔끔한 성건님 코드를 보니 확 느껴지네요!

- (임채욱)
- passport의 JwtStrategy를 설정할 때 "jwt"와 같이 이름을 지정하는 부분이 안보이는데도 사용은 "jwt"로 할 수 있네요. 저는 "jwt", new JWTStrategy 이런식으로 했었어서 여쭤봅니다.

## 회고