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