sjwoo1999/Baedal-Minjok

피드백 - 권지원 튜터

Opened this issue · 0 comments

과제 진행하느라 고생 많으셨습니다. 테스트 코드도 잘 짜셨고, DB 설계도, 3계층 분리도 잘 해주셨습니다. 에러 클래스 활용도 좋습니다.
서면 피드백은 개선점 위주로 짚어드리도록 하겠습니다.

  • 코드 컨밴션이 더 엄격하게 정해지면 좋을 것 같습니다. 라우터에서 API 경로가 mail-check, sign-up, myInfo, oneOrder 처럼 케밥 케이스와 카멜 케이스가 혼용됩니다. 컨트롤러에서 ownerRestaurant.controller.js와 error-handling.middleware.js와 ownermenus.router.js 등으로 카멜 케이스와 케밥 케이스와 플랫 케이스가 혼용됩니다. 복잡한 시스템에서는 이름을 정하는 데에도 시간이 꽤 소모되기 때문에 하나의 기준을 가지고 코드를 작성하는 것이 협업에서 중요합니다.
  • 토큰 만료 시간은 환경에 따라 조절해야하는 경우도 많기 때문에 env로 관리하는 것이 좋습니다. 환경 변수는 본질적으로 환경에 따라 변할 수 있는 변수를 넣어두는 용도이기 때문입니다.
  • DBError 같은 경우 status code 500이 더 적절할 수 있습니다. DB 서버는 웹 서버와 구분되어있고, 의도하지 않은 데이터베이스 서버 에러는 보통 500을 주기 때문입니다. 다만 데이터가 없을 때만 DBError를 주고 있기 때문에 지금 사용하신 사례로는 400번대가 어울리긴 합니다. 물론 이 사례는 데이터베이스의 에러는 아닌 것 같습니다.
  • menus.service.js의 createMenu, updatedMenu에서 validation을 재활용하는 아이디어는 별로 좋지 않아보입니다. 차라리 else { validation = false; }throw new ValidationError('자신의 식당의 메뉴만 작성할 수 있습니다.');를 넣으면 더 깔끔했을 것 같습니다.
  • menus.controller.js의 deleteMenu에서 const password = req.body;는 object이므로 if (!password) return res.status(401).json({ errMessage: '비밀번호를 입력하지 않았습니다.' });는 작동하지 않을 것 같습니다. menus 컨드롤러 테스트 코드가 있었다면 발견했을수도 있었을 것 같네요.
  • order.services.js에서 order에서처럼 for문 안에 await은 일반적으로 성능에 안좋은 영향을 줄 수 있습니다. Promise.all이나 for await ... of 문을 활용해보는 것도 좋을 것 같습니다. 그리고 한번더 reduce를 쓰지 않아도 한번의 반복문으로 총 가격을 구할 수 있을 것 같습니다.