5개월차 뉴비의 코드 리뷰

안녕하세요 백엔드팀 허민지입니다.

시간이 벌써 빠르게 흘러 비브로스에 입사한 지 약 5개월이 되어갑니다. 아직 개발자라고 자신을 소개하기에는 부끄러운 실력이지만 어느새 본격 신입 생활에 접어들었습니다. 그동안 자바스크립트와 타입스크립트를 다루며, 팀 장기 프로젝트에 투입되고 일감을 할당받아 작게나마 똑닥 서비스에 기여하고 있습니다.

똑닥 백엔드팀에는 코드 리뷰 문화가 있습니다. 코드 리뷰란 코드 작성자가 다른 개발자들의 피드백을 바탕으로 코드 내용을 수정하는 작업을 의미합니다. 이 과정에 기술적인 피드백으로 더 효과적인 코드를 작성할 수 있습니다. 또한 운영 서버 배포 전 서비스에 타격이 있을 만한 문제와 버그를 바로 잡을 수 있습니다. 백엔드팀 모두 기능 개발 후 코드 리뷰를 받으며 최소 2명의 승인을 받아야 운영에 반영할 수 있습니다. 특히 자바스크립트와 타입스크립트 초보자인 저에게 신규입사자 과제 때부터 진행되어 온 코드 리뷰는 더 효율적인 코드를 위해 고민하고 작성하는 데 정말 많은 도움이 되었다고 단언할 수 있습니다.

해당 포스팅을 통해 신규 입사자 과제부터 똑닥 서비스 투입 후 제가 받은 코드 리뷰를 간단한 예제와 함께 공유하여 많은 분들께 도움이 되고자 합니다.

코드 리뷰

1. Promise.all()

비동기 처리를 병렬적으로 순차 처리할 필요가 없다면 promise.all()으로 한 번에 처리하는 것이 더 효과적입니다.

병렬적으로 순차 처리 해야 하는 예를 들어 보겠습니다:

const value = await previousFunction();
await nextFunction(value); 

해당 예시는 previousFunction()의 반환값이 nextFunction() 함수에 필요한 경우입니다. 이처럼 순차적으로 처리해야 하는 코드의 경우 개별적으로 await을 걸어 비동기로 실행할 수 있습니다. 그러나 이처럼 병렬적으로 처리할 필요가 없으면 Promise.all()을 사용하여 처리하면 됩니다.

[코드 리뷰 전]

const buildAccount = async (fields) => { 
 const { targetId } = fields;
 // ... validation 코드 중략 
 await bigFunction(targetId).then(throwIfError());
 await megaFunction(targetId).then(throwIfError());
 // ... 다른 코드 로직
};

[코드 리뷰 후]

const buildAccount = async (fields) => { 
  const { targetId } = fields;
  // .... validation 코드 중략 
  await Promise.all([
    bigFunction(targetId).then(throwIfError()),
    megaFunction(targetId).then(throwIfError())
  ]);
 // ... 다른 코드 로직
};

2. body는 readonly로 취급

실제로 받은 리뷰...😱

예를 들어보겠습니다. 사용자가 전화번호를 입력해야 하는 구간이 있습니다. 그리고 몇몇의 사용자는 - (하이픈)을 전화번호 중간 중간에 입력한다고 가정해 봅니다. 그러나 저희는 기호 없이 전화번호의 숫자만 추출하여 데이터베이스에 저장하고 싶습니다. 그리하여 하이픈 없이 전화번호의 숫자만 return 받을 수 있는 onlyNumber()라는 함수를 작성합니다.

코드 리뷰 전 내용을 확인하면 exec() 함수는 숫자만 추출하기 위해 원본 request body의 phone 값에 onlyNumber()를 적용하여 원본 phone값을 변경합니다.

하지만 request body는 readonly로 취급해야 하며 입력값 왜곡은 절대 금지입니다. 이처럼 입력값을 활용해야 할 때 우리는 request body를 깊은 복사해야 합니다.

깊은 복사를 하는 이유는? 얕은 복사를 할 경우 원본값과 같은 메모리 주소를 바라보고 있었던 참조값도 수정 시 함께 변경될 수 있기 때문입니다. 반대로 깊은 복사를 할 경우 객체의 속성을 복사하여 별도의 메모리 주소에 저장하기 때문에 원본값을 수정해도 참조값에 영향을 주지 않습니다.

깊은 복사와 얕은 복사의 예를 들어보겠습니다:

// 얕은 복사 
const original = {
  a : 'a',
  b: 'b',
};

const diffData = original; 

original === diffData;  // true

original.a = 'change'; // original.a = 'change'

diffData.a;            // diffData.a = 'change' -> original 처럼 a값이 수정됨

// 깊은 복사 
const original = {
  a : 'a',
  b: 'b',
};

const diffData = {
  ...original,
};

const diffData2 = _.cloneDeep(original);

original === diffData;  // false

깊은 복사를 하기 위해서는 lodash의 _.clonedeep() 메소드 또는 spread 연산자를 사용합니다. lodash 메소드와 다르게 spread 연산자는 1-depth까지만 복사되기 때문에 주의해야 합니다.

[코드 리뷰 전]

 // 전화번호의 숫자만 추출하여 저장하는 함수 
 const onlyNumber = (str: string): string => str.replace(/[^\d]/g, '');

 const exec = async (body) => {
   // ... doing something 
   body.phone = onlyNumber(body.phone);
 };

[코드 리뷰 후]

 import * as _ from 'lodash';

 // 전화번호의 숫자만 추출하여 저장하는 함수 
 const onlyNumber = (str: string) => str.replace(/[^\d]/g, '');

 const exec = async (body) => {
   const queryClone = _.cloneDeep(body);
   const { name, phone, address } = queryClone; 
   await UserMode.create({
     phone: onlyNumber(phone),
     ...
 };

3. return의 묘미

• 바로 return 하기

  필요없는 변수는 만들지 않습니다. 바로 return 할 수 있는 코드는 바로 return 합니다.

  [코드 리뷰 전]

  const createUser = async (name, email) => {
    // ... validation 코드 중략
    const newUser = await UserModel.create({
      name,
      email,
      createdAt: new Date(),
      updatedAt: new Date(),
    });
    return newUser;
  };

  [코드 리뷰 후]

  const createUser = async (name, email) => {
    // ... validation 코드 중략
    return UserModel.create({
      name,
      email,
      createdAt: new Date(),
      updatedAt: new Date(),
    });
  };

• return? return await?

  에러를 추후 handling 하는 로직도 없으며, 사용자 또한 비동기 처리 과정에 대한 문제 발생 여부를 알 필요가 없다고 가정해봅니다. 그럼 return-await이 아닌 바로 return을 해 백그라운드에서 비동기 처리하도록 합니다.

  [코드 리뷰 전]

  const postOrder = async (body) => {
    const { userId, name, address, phone } = body;
    // ... validation 코드 중략
    return await OrderModel.create({
      userId,
      name,
      address,
      phone,
      createdAt: new Date(),
      updatedAt: new Date(),
    });
  };

  [코드 리뷰 후]

  const createUser = async (body) => {
    const { userId, name, address, phone } = body;
    // ... validation 코드 중략
    return OrderModel.create({
      userId,
      name,
      address,
      phone,
      createdAt: new Date(),
      updatedAt: new Date(),
    });
  };

  💥 [F/U] 최근 nodebestpractices에서 return await을 진행하는 것이 에러 스택 추적에 도움된다고 합니다. 자세한 내용은 해당 링크를 통하여 들어가시면 확인하실 수 있습니다.

4. try-catch 블록은 작게 잡기

try 블록 안에 여러 번의 비동기 작업이 진행된다고 가정해봅니다. 그 중 하나에서 에러가 발생하였습니다. 그러나 catch 블록을 통해 에러가 잡혀도 우리는 해당 에러가 어떤 비동기 처리 작업에서 발생했는지 알 수 없습니다. 이때 try-catch 블록 대신 개별적으로 promise 뒤에 catch()를 추가하거나, error가 정확히 어디서 발생했는지 알기 위해 try-catch 블록을 작게 잡습니다.

[코드 리뷰 전]

const createUser = async (body) => {
  try {
    const { name, email } = body;
    // ... validation 코드 중략
    const result = await someFunction(body);
    const resultTwo = await otherFunction(body);
    return someOtherFunction(result, resultTwo);
  } catch (error) {
    throw new Error('Oops');
  }
};

[코드 리뷰 후]

const createAccount = async (body) => {
  const { name, email } = body;
  // ... validation 코드 중략
  const result = await someFunction(body).catch((err) => {
    throw new Error('Oops from someFunction');
  });
  const resultTwo = await otherFunction(body).catch((err) => {
    throw new Error('Oops from otherFunction');
  });
  return someOtherFunction(result, resultTwo).catch((err) => {
    throw new Error('Oops from Final Function');
  });
};

5. 메세지, 입력값 검증 파일 분리

에러 반환 메시지 및 입력값 검증하는 함수는 별도의 파일을 생성해 service 파일과 분리합니다.

[코드 리뷰 전]

const createUser = async (body) => {
  const { name, email } = body;
 
  if (_.isNil(name) || _.isEmpty(name)) {
    throw Error('이름 누락되었습니다.');
  }
 
  if (_.isNil(email) || _.isEmpty(email)) {
    throw Error('이메일 누락되었습니다.');
  }
 
  const result = await someFunction(body).catch((err) => {
    throw new Error(err);
  });
  
  return someOtherFunction(result).catch((err) => {
    throw new Error(err);
  });
};

[코드 리뷰 후]

// user.message 파일
export const message {
  NAME_REQUIRED: '이름 누락되었습니다.',
  EMAIL_REQUIRED: '이메일 누락되었습니다.',
}

// user.validate 파일
import * as message from './user.message';
  
export const validateName = (name) => {
  if (_.isNil(name) || _.isEmpty(name)) {
    throw new Error(message.NAME_REQUIRED)
  }
};

export const validateEmail = (email) => {
  if (_.isNil(email) || _.isEmpty(email)) {
    throw new Error(message.EMAIL_REQUIRED)
  }
};
  
// user.service 파일
import * as validate from './user.valiate';

const createUser = async (body) => {
  const { name, email } = body;
  
  validateName(name);
  validateEmail(email);
    
  const result = await someFunction(body).catch((err) => {
    throw new Error(err);
  });
  
  return someOtherFunction(result).catch((err) => {
    throw new Error(err);
  });
};

6. 알잘딱깔센 함수명, 변수명

함수 또는 변수는 해당 역할, 수행 목적과 맞는 이름이어야 합니다. 정확한 함수/변수명일수록 코드 리뷰어도 더 효과적으로 내용을 이해하고 피드백을 줄 수 있습니다. (추후 해당 코드를 수정할 분께도 자비를...)

[코드 리뷰 전]

// 불명확한 함수명
const func = (body) => {
  // 불명확한 변수명
  const a = 18;
  if (body.age < a) {
    throw new Error(`만 ${a} 미만은 일반 회원가입이 불가능합니다.`)
  }
  return UserModel.create({
    name: body.name,
    email: body.email,
    age: body.age,
    createdAt: new Date(),
    updatedAt: new Date(),
  });
};

[코드 리뷰 후]

// 명확한 함수명
const createUser = (body) => {
  // 명확한 변수명
  const AGE_LIMIT = 18;
  if (body.age < AGE_LIMIT) {
    throw new Error(`만 ${AGE_LIMIT} 미만은 일반 회원가입이 불가능합니다.`)
  }
  return UserModel.create({
    name: body.name,
    email: body.email,
    age: body.age,
    createdAt: new Date(),
    updatedAt: new Date(),
  });
};

7. 주석은 매너

큰 함수나 인터페이스 같은 경우 주석을 통해 해당 함수의 역할과 목적을 표기합니다. 주석은 적절한 키워드 및 설명이 포함하여 있어야 합니다.

[코드 리뷰 전]

const createUser = (userInfo) => {
  const AGE_LIMIT = 18;
  if (userInfo.age < AGE_LIMIT) {
    throw new Error(`만 ${AGE_LIMIT} 미만은 일반 회원가입이 불가능합니다.`)
  }
  return UserModel.create({
    name: userInfo.name,
    email: userInfo.email,
    age: userInfo.age,
    createdAt: new Date(),
    updatedAt: new Date(),
  });
};

[코드 리뷰 후]

/**
 * 일반 회원가입 사용자 생성
 *
 * @param userInfo - 생성하고자 하는 사용자 정보
 */
const createUser = (userInfo) => {
  // 사용자 나이 제한
  const AGE_LIMIT = 18; // 상수끼리 모아둔 파일로 분리할 수 있겠죠?
  if (userInfo.age < AGE_LIMIT) {
    throw new Error(`만 ${AGE_LIMIT} 미만은 일반 회원가입이 불가능합니다.`)
  }
  return UserModel.create({
    name: userInfo.name,
    email: userInfo.email,
    age: userInfo.age,
    createdAt: new Date(),
    updatedAt: new Date(),
  });
};

마무리

5개월 동안 받았던 코드 리뷰를 하나하나 확인해보니 저의 코드가 5개월간 진화(?)했다는 것을 느낄 수 있었습니다. 저는 오늘도 코드 리뷰를 통해 더 효율적인 코드를 작성하고, 똑닥 서비스 및 아키텍처 전반을 세심하게 살피고 있습니다. 이처럼 똑닥 백엔드팀은 코드 리뷰 문화를 통하여 더 나은 서비스를 위해 함께 고민하고 기술을 공유합니다.

긴 글 읽어주셔서 감사합니다.

비브로스는 현재 채용 진행중입니다

더 자세한 내용은 해당 링크에서 확인하실 수 있습니다:

• 원티드
• 비브로스