[팀스터디] 남의 팀 프로젝트 뜯어보기 (가까운4이)

https://github.com/muja-code/closer4u_service

GitHub - muja-code/closer4u_service: closer4u_service

 

들어가기 앞서

기본 구조나 전체적으로 코드의 퀄리티가 좋아 

굳이? 싶은 부분들에 대해 좀 더 민감하게 따지고 들어갔던 것 같다.

하지만 경험상 이런 사소한 차이가 나중에 더 확장이나 유지보수하기 편했었기에 

그런 부분들도 모두 기록해보았다.

 

아래 의견들은 모두 내 주관에 따라 적었으므로 틀린 부분이 있다면 님 말이 맞음

 

 

아쉬운 부분

폴더 구조

전체 폴더구조가 하나의 depth로 모두 나와있는 상태라 원하는걸 폴더를 특정해 찾기는 쉽지만

유형별(성격별)로 구분되어 있지 않아 하나의 주제로 찾기엔 흩어져 있는 모습을 볼 수 있다.

ex) sequelize 관련 폴더

 

Router 네이밍

app.js

const router = require('./routes/index');
const pageRouter = require('./routes/page');

app.use('/api', router);
app.use('/', pageRouter);

apiRouter, pageRouter로 구분했으면 더 좋지 않았을까

routes/index로는 어떤 라우터인지 구분 안갈수도

 

req.userInfo 사용 이유

auth-token.js - Ln 32

req.userInfo = jwt.verify(accessToken, process.env.ACCESS_JWT_SECRET_KET);

req.userInfo에 넣은 이유가 있는지?

controller가 아닌 곳에서도 사용하려고 하나 했지만 그것도 아니었음

강의에서는 res.locals.userInfo와 같은 방식으로 넣는게

next()해서 보낸 곳에서만 사용이 가능해져서 휘발성이 있는 점을 장점으로 사용한다고 배웠음

 

FK 설정 - 확인 후 삭제

// models/User.js
class User extends Model {
    static associate(models) {
      User.hasMany(models.Order, { foreignKey: 'user_id' });
      User.hasMany(models.Review, { foreignKey: 'order_id' });
    }
  }
// models/Order.js
class Order extends Model {
    static associate(models) {
      Order.hasMany(models.Review, { foreignKey: 'order_id' });
      Order.belongsTo(models.User, { foreignKey: 'user_id' });
    }
  }
// models/Review.js
    class Review extends Model {
    static associate(models) {
      Review.belongsTo(models.User, { foreignKey: 'user_id' });
      Review.belongsTo(models.Order, { foreignKey: 'order_id' });
    }
  }

종속 관계 세팅은 되어있는거 같은데 migration 한 DB상에서는 안되어 있음. 

추가적으로 세팅해야 되는 부분인지

 

signup - login

sigh up - sigh in

register - login

등의 조합으로 쓰인다고 함.

외국의 경우 전자, 한국의 경우 후자를 자주 사용하는 것으로 보임



date.js의 존재

백엔드에서 프론트엔드가 사용하기 좋게 가공해줄 필요가 있는지

 

ejs 파일에 노출된 css 코드

head.ejs

review.css 처럼 따로 빼지 않은 이유

 

로그아웃 버튼

document.addEventListener("DOMContentLoaded", () => {
  const logout = document.getElementsByClassName('logout');
  for (let i = 0; i < logout.length; i++) {
    logout[i].addEventListener('click', () => {
      axios({
        method: 'post',
        url: '/api/users/logout',
        data: {}
      }).then((response) => {
        window.location.href = '/'
      })
    })
  }
})

1. 바로 실행되지 않을 함수는 굳이 DomContentLoaded로 넣지 않아도 됐을듯
2. 어차피 if문 분기로 하나만 생성될 로그아웃 버튼인데 class로 구분해서 for문을 돌릴 필요가 있었을까 하는 생각

document.querySelector('#logout').addEventListener('click', () => {
  axios({
    method: 'post',
    url: '/api/users/logout',
    data: {}
  }).then((response) => {
    window.location.href = '/'
  })
})

내가 작성했다면 이렇게 했을듯

 

User의 member 값

member라는 이름만으로 일반고객, 사업체에 대해

0, 1로 구분 짓겠구나라고 유추하기 쉽지 않아 보임

차라리 isAdmin true, false가 더 나을듯

 

예외처리

getOrderRequests = async () => {
    try {
      // ...생략
      if (!orders) {
        throw new Error('Order Error');
      }
      // ...생략
    } catch (error) {
      console.log(error);
      return error;
    }
  };

예외처리가 미숙한 점이 아쉽

 

깨알 영어 공부

datas는 안쓰는걸 추천

data는 복수형. datum이 단수형이라고 하지만 최근에는 거의 쓰지 않으며 

data를 단수형으로도 쓰기도 한다고 한다.

 

res.render와 res.json이 같이 쓰이는 형태

controllers/orders/orders.js

getOrderRequests = async (req, res, next) => {
    try {
      // ...생략
      res.status(200).render('order-requests', {
        // ...생략
      });
    } catch (error) {
      res.status(400).json({ errorMessage: '요청이 올바르지 않습니다.' });
    }
  };

보통의 상황에서는 res.render()로 페이지 이동을 하는데

에러나는 상황에서는 catca문의 res.json()로 응답하는 경우가 있다.

이러면 프론트엔드에서 혼동이 있을듯

 

jwt expiresIn 하드코딩

내 팀프로젝트 코드를 리뷰하면서도 말했던 내용이지만 

이 부분은 env 설정을 통해 세팅하는게 나중에 상용으로 배포되었을 때 컨트롤하기 쉽다.

 

service에서의 db 조작

UserService.signupUser 메소드

const duplicateUser = await User.findAll({

repo에 전달하기 위해 호출한 model을 직접 사용하고 있음

 

신청 시에 닉네임, 주소 추가 기입

내 정보가 있음에도 닉네임, 주소를 적게 해둔건

변경해서 신청할 경우를 위한것인지?

신청 폼에서 그럼 기존 데이터가 나와야 할듯

 

이미지 저장 시 한글 깨짐 현상

닉네임-날짜-파일명 으로 저장되는거 같은데 

입력받은 닉네임은 한글로 잘 저장되지만

파일명 부분이 한글이면 다 깨져서 저장되는 현상 발견

같은 상황 겪고 해결해봐서 관련 링크 참고

https://4sii.tistory.com/304

[multer] 한글 깨짐 현상 해결 방법

 

주문신청내역에 다른사람 것도 보임

개인유저1이 주문 신청한게 개인유저2로 로그인 시 보이는 현상

 

+ 최신화된 코드에서

인증 오류는 정확하지 않게

아이디가 잘못됐는지, 비밀번호가 잘못됐는지 정확히 알려주는 방법은 

보안상 위험하므로 둘 다 합쳐서 '로그인 실패' 정도의 뉘앙스를 주게끔

 

 

좋았던 부분

폴더 구조

내 프로젝트 포함 지금까지 봐온 프로젝트중에서 제일 이해하기 쉬운 프로젝트 폴더 구조였다.

덕분에 헷갈리지 않고 모든 코드들에 대해 리뷰가 가능했음

 

ejs에서의 try - catch문 활용

• head.ejs: nav를 만들 때 이런식으로도 사용할 수 있구나를 배움
• order-list.ejs: 목록을 구성하는 부분에서도 try - catch문을 이용해
  손쉽게 데이터 없는 상황에 대한 컨트롤을 보여주었다.

 

axios 사용으로 인한 코드의 깔끔함

나는 전 프로젝트에서 Vanilla JS의 장점을 열거하며

반강제로 fetch를 쓰게 유도했는데 (반성.. 🙏)

axios를 씀으로써 매우 쉽고 간결한 코드로도 백엔드 서버와 통신이 가능하게끔 만들어

팀을 리딩함에 있어 팀원들에게 이런 코드 사용법을 공유하는게 더 좋겠다는 생각이 들었다.

 

ejs 코드 가독성

ejs 코드도 prettier 쓴 것 처럼 모두 균일하게 깔끔한 것을 보았는데

따로 사용한 라이브러리가 있는지?

 

bcrypt와 async/await 사용

내 코드

encryptPassword = async (password) => {
  return new Promise((resolve, reject) => {
    bcrypt.hash(password, saltRounds, (err, hash) => {
      // ...생략
    });
  });
};

가까운4이조 코드

const encryptPassword = await bcrypt.hash(password, saltRounds);

bcrypt의 hash와 compare 함수에 대해 사용하는 방법이 두 가지 있는데

return을 Promise로 받아 await하게 사용하는 방법을 써서 아주 깔끔하게 사용할 수 있던 모습을 볼 수 있다.

덕분에 overload 되는 함수인 걸 알게 되었고 

내 프로젝트에서도 개선을 할 수 있었다.

 

DB 컬럼에 대해 저장 공간 준 부분

sequelize migration 파일에서 부터 저장 공간을 설정할 수 있는걸 몰랐는데

password: {
  type: DataTypes.STRING,
},
nickname: {
  type: DataTypes.STRING(100),
},

이런 방법으로 줄 수 있다는걸 배웠다.

하지만 nickname이나 phone이 100글자까지 되진 않을텐데 varchar(100)으로 준 부분은 아쉬웠음

 

+ 최신화된 코드

multer Class 분리

multer.js

const multer = require('multer');
const path = require('path');
const dayjs = require('dayjs');

const imagePath = path.join(__dirname, '../', 'public', 'images');

const imageStorage = multer.diskStorage({
  destination: (req, file, cb) => {
    cb(null, imagePath);
  },
  filename: (req, file, cb) => {
    req.body.image =
      req.body.nickname +
      '-' +
      dayjs().format('YYYY-MM-DD-HH-mm') +
      '-' +
      file.originalname;
    cb(
      null,
      req.body.nickname +
        '-' +
        dayjs().format('YYYY-MM-DD-HH-mm') +
        '-' +
        file.originalname
    );
  },
});
const upload = multer({ storage: imageStorage });

module.exports = upload;

routes/orders/order.js

const upload = require('../../utills/multer');

router.post(
  '/',
  authToken,
  upload.single('image'),
  ordersController.createOrders
);

 

내 코드

UploadManager.js

'use strict';

const multer = require('multer');
const moment = require('moment');
const fs = require('fs');

class UploadManager {
  constructor(path) {
    this.path = path;
    this.multer = multer;
  }

  storage = multer.diskStorage({
    destination: (req, file, cb) => {
      const upload_path = this.path;
      fs.mkdirSync(upload_path, { recursive: true });
      cb(null, upload_path);
    },
    filename: (req, file, cb) => {
      file.originalname = Buffer.from(file.originalname, 'latin1').toString('utf8');
      cb(null, moment().format('YYYYMMDDHHmmss') + '_' + file.originalname);
    },
  });
}

module.exports = UploadManager;

orders.routes.js

const UploadManager = require('../config/UploadManager');
const uploadManager = new UploadManager(process.env.MULTER_PATH_UPLOADS_ORDERS);

router.post(
  '/',
  authMiddleware,
  uploadManager.multer({ storage: uploadManager.storage }).array('files'),
  ordersController.createOrder
);

내가 모듈화한 방식과는 또 다른 방법으로 모듈화한걸 보면서 두 코드의 장점을 살려

다음 프로젝트 때 쓰면 좋을거 같아 기록

 

logging 시스템 winston 적용

날짜까지 구분되어 저장되는 부분 굿

 

 

궁금한 부분

form 태그 사용

나는 form 태그 사용할 때마다 잘 안됐던 기억이 있어서

웬만한 상황에서 document.querySelector를 이용해 각각의 input에서 값을 빼와 사용했는데

form 태그로 데이터를 보낸다면 추가적인 JavaScript 코드 작성 없이도 api 요청이 가능할듯

 

하지만 이럴 경우 input value에 대한 유효성 확인을 백엔드에서 해줘야 하는 상황이 벌어진다.

프론트엔드에서 확인하려면 어쨌든 JavaScript 단에서 확인이 필요하게 됨

 

그리고 백엔드에 대한 응답을 받아야 하는데 받는 것으로 보이는 코드가 안보임

회원가입을 했는데 이게 잘된건지 안된건지 모르겠는 상황이 연출된다.

 

파일명 관련

users 관련 기능에 대해

controllers, services, repositories에서 모두 users.js 파일명 등을 썼는데

작업하면서 불편하지 않았는지

나 포함 다른 팀들은 보통

• Users.controller.js
• Users.service.js
• Users.repository.js

위와 같은 방식으로 사용했기에 궁금한 부분

그리고 class 파일명은 대문자로 시작해야 하는게 규칙이다. 

 

사용되지 않는 값

services/users.js

if (duplicateUser.length) {
  return {
    code: 400,
    errorMessage: '이미 가입된 아이디 또는 닉네임이 있습니다.',
  };
}

return 으로 code 값을 담아 넘기는데

controllers/users.js 에서 사용하는 곳은 없음

 

튜터 피드백

입력값 유효성 검증

공통적으로 지적받은게 controller에서 전달받은 req.body에 대한 유효성 검증이 필요하단 부분이었는데

어떤식으로 처리할건지?

 

에러 Class 관리

그냥 new Error로 에러 객체를 만드는게 아닌 상속받는 새로운 Error Class 만드는걸 추천했는데

그 부분에 대해서도 새로 프로젝트 할 때 고려해보면 좋을듯

 

await headBox()가 뭐지

order_requests.js에 나오는 코드

 

 

마치며

전체적으로 봐온 프로젝트중에 제일 깔끔하고 보기 좋았으며

기능들에 대해서도 리뷰하면서 내내

'아 팀프로젝트 리뷰하는 스터디 하길 정말 잘했다'라는 생각이 들 정도로 너무 좋았다.

 

해당 프로젝트를 만든 사람이 같이 캠프에서 공부하는 사람이라 

내가 이렇게 리뷰한걸 알려주면서 그 분도 부족한 부분을 배우고

나도 내 코드에서 수정되어야 하거나 추가되어야 하는 부분들이 있다는 걸

이번 리뷰를 통해 많이 배운 것 같다. 

 

이번이 나한테 있어서 두 번째로 남의 프로젝트를 리뷰하는 시간이었는데

리뷰할 때마다 내 경험치가 늘어나 리뷰도 잘하게 되고 

내 코드 실력도 향상됨을 느꼈다. 

 

앞으로도 기회가 된다면 다른 팀의 프로젝트를 리뷰하거나 

아예 깃허브에서 궁금한 프로젝트를 까보거나 하는 식으로도 성장할 수 있을 것 같다. 끝

 

 

 

+ 리뷰 후

res.locals.userInfo 를 썼으나 에러 발생해서 userInfo를 찾지 못함
-> req.userInfo를 사용한 이유

db 테이블 종속 안한 이유
시퀄에서 다 해주니까
굳이 데이터베이스에 안해도 
models에만 설정해주면 되긴 함
-> 디비쪽도 같이 해주는게 장기적으로 좋을듯

ejs는 html로 인식
vscode 자체적으로 format html 관련해서 적용했음
탭 2로 적용


클래스 파일명에 . 으로 구분짓는거 이유 찾기
users.controller.js 같은

headbox() 
헤더에서 메뉴바들이 동시에 작동하다보니까