안녕하세요 백엔드팀 허민지입니다.
시간이 벌써 빠르게 흘러 비브로스에 입사한 지 약 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개월간 진화(?)했다는 것을 느낄 수 있었습니다. 저는 오늘도 코드 리뷰를 통해 더 효율적인 코드를 작성하고, 똑닥 서비스 및 아키텍처 전반을 세심하게 살피고 있습니다. 이처럼 똑닥 백엔드팀은 코드 리뷰 문화를 통하여 더 나은 서비스를 위해 함께 고민하고 기술을 공유합니다.
긴 글 읽어주셔서 감사합니다.
비브로스는 현재 채용 진행중입니다
더 자세한 내용은 해당 링크에서 확인하실 수 있습니다: