[팀프로젝트#3] 키보드를 구해조 - 피드백 정리

팀프로젝트 피드백 정리

팀프로젝트 <키보드를 구해조!> - [이슈 링크]

GitHub - 9hezo/save_my_keyboard

 

바로 적용 가능한 피드백

 

# 27 장황한 코드 정리 - 한줄로 나타낼 수 있는 코드는 한줄로
-> 띄워놓은 github 참고
https://github.com/9hezo/save_my_keyboard/commit/43b222603d4cf7a4eae3ebfb100beaeb0deaeee3

# 29 백엔드 / 프론트엔드 분리
-> res.render 등을 통해 페이지 이동할 경우 render에 백엔드가 개입해서 db의 값을 사용하는건 좋지않음
백엔드의 데이터는 json 으로만 응답 
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/controllers/reviews.controller.js#L13

#30 변수명 
-> true/false 와 같은 결과가 나오는 변수/함수명은 isSomething 과 같이 앞에 is를 붙이는게
명확하다. 
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/controllers/users.controller.js#L9

# 31 쿠키를 생성하는 주체
-> users.service 에서 res에 등록해버리는게 아닌 토큰을 프론트에 전달해서
거기서 저장하게끔
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/controllers/users.controller.js#L32

+ # 32 리팩토링 (해빈님)
-> 응답에 필요없는 프로퍼티들은 애초에 디비에서 빼올 때 제외하고 가져오기
sort()의 임시 인자로 쓰이는 값들도 단순한 a, b가 아닌 의미있는 변수명 부여
ex) a, b -> prev, next
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/services/orders.service.js#L13
ex)
    return allLists.map(orders => {
      const { 응답에 필요없는 properties, ..output} = orders
      return orders
    }).sort((prev, next) => {
      return prev.createdAt - next.createdAt;
    });


# 34, # 35, # 36
-> if문 안의 if문 같은 중복 조건문은 사용 지양
그리고 애초에 오류나 에러 상황에 대한건 미리 캐치 후 실패 반환
성공에 대한 리턴은 맨 마지막 줄에 오게끔 하는게 베스트
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/services/orders.service.js#L68
ex)
    if (createResult === 0) {
      return {code: 400, message; '에러'}
    }

   const pointDeductResult = await this.ordersRepository.pointDeduct(ownerId, process.env.ORDER_PRICE);
   if (!pointDeductResult) {
         return {code: 400, message; '에러'}
   }
  return { code: 201, message: '주문에 성공하였습니다.' };



# 36 - 공통 종합, # 39, # 41, # 42
-> console.log 는 로직 설명이 아닌 경우엔 모두 제거 
-> 확인해야 되는 console.log면 winston과 같은 logger 시스템 사용
-> 수정/ 삭제 시 진짜 삭제해도 되는 상황인지에 대한 유효성 검사 필요 
-> 해당 사용자인지, 잔액은 충분한지 등
-> 에러 처리 단순히 -1이나 console.log('실패')와 같이 하지 않고 적재적소에 잘 하기
-> throw 하는게 어떨까
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/repositories/orders.repository.js#L38
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/repositories/tokens.repository.js#L20

# 37 - 프로미스 사용한 이유
-> 프로미스 쓰는 이유에 대해 설명 및
-> 프로미스를 썼으면 async, await을 써서도 똑같은 기능이 동작되게 할 수 있는지
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/services/users.service.js#L78
-> 변경 후에 따른 변화 추가 예정

# 40 함수명
-> 나만, 혹은 팀원만 알아볼 수 있는 함수명이 아닌 누구나 알아볼 수 있고
용도를 확인할 수 있는 함수명 사용
ex) getOrderStatusZeroToThree : status가 0에서 3인 order를 가져오는 함수: 상태가 진행중
-> getOrderWaiting
https://github.com/9hezo/save_my_keyboard/blob/3d43b9d533b095ba96c5257ff68d7cf3e5183906/app/src/repositories/orders.repository.js#L42

추가로 확인 후 적용 가능한 피드백

# 28 인자 갯수 제한 방법

https://github.com/9hezo/save_my_keyboard/issues/28

 

before

create()의 인자로 넣을 때 

key값과 value에 들어갈 변수명이 같으므로 생략되는 효과를 이용해 

간결하고 보기좋은 코드가 될 수 있음

 

after

orders.service.js

orders.repository.js

인자가 4개 이상일 경우 객체로 넘겨주기 위해 orderInfo에 넣어 사용한 모습.

앞뒤 생략한 ownerId 하나로 사용 가능했던게 ownerId: orderInfo.ownerId 와 같이 길어졌다.

 

해결방법?

객체 구조 분해 할당을 사용했을 때 위와 같이 나타낼 수는 있음

 

확실히 인자 개수가 많아질수록 관리하기도 힘들고 보기도, 찾기도 힘든건 알겠는데

repo에서 제일 마지막 예와 같이 사용하는건 작성할 때는 좋지만

보기 힘들다는건 동일하다. (대신 repo 전 까지는 orderInfo에 담겨 이동되기 때문에 좋음)

 

이 경우 어떻게 사용하는게 나중 갔을 때 좀 더 효율적일지 좀 더 편할지 궁금

 

답변:

정답은 없다. 하나의 컨벤션일 뿐 팀이나 조직 또는 개인이 추구하는 스타일대로 하는게 맞음

정답은 없지만 여러 회사의 컨벤션을 참고하는 것도 좋은 방법이다.

https://angelplayer.tistory.com/111

[JS] 자바스크립트 코딩 컨벤션 (Naver, Airbnb, standardJS)

 

# 33 status 값에 대해 무엇을 의미하는지 알아볼 수 있는 값으로 사용

-> 다른 팀 프로젝트는 어떤식인가 먼저 참고하고 물어보자

https://github.com/9hezo/save_my_keyboard/issues/33

 

-> 다른팀도 마찬가지로 status에 숫자값을 사용했음 

이슈 코멘트에 작성한 것 처럼

1. varchar로 상태명을 한글로 사용? ex) wainting_0, cancel_5
2. http 상태 코드처럼 0~5가 아닌 좀 더 체계적인 100 101, 200과 같은 넘버를 사용?

두가지 방안에 대해 생각해봤는데 현업에선 어떤 방식으로 사용하는지 궁금

 

 

답변: 

그냥 문자열로 쓰는게 맞음 

ex) waiting, collecting 등등

keyboard_collecting 또는 

collectingKeyboard 와 같이 쓰는건 비추천

애초에 뭘 수거할거냐 라는 선택지에서 우리가 수거할건 키보드 밖에 없기 때문에

생략이 가능하니까

 

아예 한글명으로 써버리는 것도  하나의 방법.

실제로 직방 다니실 때 직방에서는 한글 상태값을 사용했는데 문제될건 없었다.

옛날이라야 인코딩이 깨질까 걱정됐다지만 이제는 해당되지 않음

 

status 값 만으로 프로세스를 확인할 수 없는 경우에는 

기획서나 정책 문서에 정의가 되어 있고 그걸 참고하는게 맞음

운영 과정에서 프로세스가 바뀔 수도 있고 새로운 status가 추가될 수도 있다.

그럴 경우 하나씩 다 땡기거나 늘려야하는지? (사진참고)

대기중 - 배송 - 수거 - 이런식으로바뀌게 되도 혼돈의 카오스

 

또한, 숫자로 되있을 경우 나중에 고쳐야 될 때 해당 숫자 ex) 0, 1, 2, 3, 4, ...

를 찾아서 바꿔야 하는데 이 숫자들을 검색했을 때 검색 결과가 어떻게 나올까? -> HELL

 

 

# 36 공통 종합

-> middleware와 util 파일들을 config폴더가 아닌 따로 나누는게 좋을지?

https://github.com/9hezo/save_my_keyboard/issues/36

 

답변:

config는 설정 파일이나 공통 환경 변수에 대한 내용이 들어가 있는게 맞다.

때문에 config라는 폴더의 존재 자체가 애초에 옳지 못하고

src/middlewares

src/utils 

폴더를 만들어 구분해야 좋을듯

 

참고로 util과 service의 차이를 명확히 구분해야 하는데

service는 비즈니스 로직에 실제로 영향을 주는 파일들이고

util은 말 그대로 도움을 주는 (ex 숫자가 소수로 나오면 소숫점을 다 빼주는 기능 같은 것들)이다.

 

# 38 불필요한 두줄 사용 지양

-> return await ~~ 인데 맨 마지막에서 처리하고 있으니
-> 굳이 await을 할 필요가 없어서 (기다릴 필요 없으니까) 생략 가능한건지?

https://github.com/9hezo/save_my_keyboard/issues/38

 

답변: 

함수가 리턴될 때는 결과값이 있어야 리턴하는데 위 코드에는 그게 없음

그리고 return 할 때 await 까지 생략이 가능하다.

 

하지만 findOne(위 링크의 코드 참고) 안에서 에러가 날 경우 

await일 경우 에러를 던져줌

생략했다면 에러를 던져주지 않고 promise의 reject를 넘겨준다고 한다. 

이 부분은 확실치 않으니 따로 확인이 필요

https://ooeunz.tistory.com/47

[Node.js] await vs return vs return await: 비동기 이해하기

하지만 이 방법에 대해서도 여러 컨벤션이 있으니 정답은 없다.

 

 

그리고 이 이슈의 불필요한 두줄 사용과

이번 포스팅의 제일 첫 번째 이슈인 # 27의 한줄로 나타낼 수 있는 코드는 한줄로 나타내기

의 공통점은 test code에서 coverage를 받을 때 영향이 있다는 것이다.

 

coverage가 100인 두줄 짜리 코드가 3줄로 늘어난다면 66이 될 것이다.

 

이처럼 테스트 코드적인 관점에서 보더라도 코드를 줄일 수 있다면 줄이는게 좋다.

물론 이것 또한 개인의 주관적인 의견이라고 하셨다.