January 25, 2020
작년 발표에서 ‘지식이 많다고 해서 반드시 좋은 코드를 쓸 수 있는 것은 아니다.‘라고 했다. 물론 아는 것이 많아지면 좋은 코드를 작성할 확률이 높아질 수는 있다.
하지만, ‘좋은 코드’라는 단어는 지식 말고도 고려해야 할 점이 많은 단어이다.
이 글은 내가 생각해본 좋은 코드에 대해 소개하는 글이다.
‘좋다’라는 문장에서 알 수 있듯, 주관적 생각일 뿐이다. 시간이 지나면 변할 수 있는 생각이기 때문에 ‘2020version’이라고 소개한다.
함수를 새로 작성할 때 이런 고민에 부딪힌다.
각각의 고민들은 얼마나 유의미할까?
결론부터 말하자면, 범용적 util과 일회성 util을 착각하지 않아야 한다. 기본적인 Data를 다루는 util일수록 재사용할 확률이 높고, Component에서 필요해서 만든 util일수록 일회성으로 사용될 확률이 높다.
export const padZero = (num: number, width: number) => {
const numToString = num.toString()
const padNumber: number = width > 0 ? width : numToString.length
return numToString.length >= padNumber
? numToString
: new Array(padNumber - numToString.length + 1).join('0') + num
}
padZero(2, 4)
와 같이 사용하면 ‘0002’를 return 하는 함수이다.
이런 기능은 어떤 Domain에서 사용될까?
쉽게 예측할 수 없다. 숫자에 padding을 넣는 작업은 꼭 지금 작업하고 있는 부분이 아니더라도 많이 일어날 수 있는 일이다.
let Directions = {
top(...) {
// 5 unique lines of math
},
left(...) {
// 5 unique lines of math
},
bottom(...) {
// 5 unique lines of math
},
right(...) {
// 5 unique lines of math
},
};
위 예시는 Dan Abramov의 Goodbye, Clean Code에서 가져온 예시이다.
Directions.top
은 ‘도형의 가장자리를 드래그하여 크기를 조절할 수 있게 해주는 기능’을 위해 만들어진 함수이다. 지금은 네 가지 방향밖에 없지만 혹시 더 늘어나게 된다면 어떤 일이 일어날까?
Goodbye, Clean Code에선 이렇게 말하고 있다.
“요구사항이 바뀌어 각각의 도형, 각각의 동작에 대한 예외 케이스가 여럿 생겼다고 가정해보겠습니다. 제가 만든 코드는 더 깊은 추상화를 만들어 내야 하지만, 기존의 ‘지저분한’(추상화 하기 이전) 버전의 코드는 매우 쉽게 변경을 반영할 수 있을 것입니다.”
‘언젠가는 사용되지 않을까’라는 희망으로부터 비롯된 성급한 추상화는 현재 이 기능을 리뷰할 동료에게 고민과 이해의 시간을 갖게 만든다. 더 나아가면, 높은 추상화 레벨에 맞추느라 초기에 얻었던 추상화로 인한 깔끔함은 아예 사라질 수 있다.
성급한 추상화는 경계해야 한다.
redux-middleware관련 util을 만들 때 많이 했던 생각이다. 비동기 로직을 처리하는 각각의 middleware들은 action의 type만 다를 뿐 다른 부분은 거의 비슷했다. 매번 모든 코드를 중복으로 적어주는 일이 꽤나 귀찮아졌다.
그래서 fetchMiddleware
라는 이름의 util을 만들었다.
// fetchMiddleware.ts
const fetchMiddleware = (action, fn?: (...args: any) => any) => {
// action payload를 통해 api call과 error를 처리하는 로직
}
그리고 팀원들에게 전파한다. “middleware로직을 하나의 함수로 사용할 수 있도록 했으니 가져다 쓰시기만 하면 됩니다! 여러 가지 처리를 하느라 내부는 조금 복잡합니다.”
이 util을 처음 전달하는 순간에는 모두가 행복할 수도 있다.
하지만 아예 프로젝트의 담당자가 바뀐다면 어떻게 될까? 에러 상황에 대한 디버깅이나 근본적인 구조를 건드려야 하는 일이 생긴다면 동료는 필연적으로 내가 만든 fetchMiddleware
도 살펴보게 될 것이다.
긴 시간을 들여야만 이 util이 이해된다는 것은 프로젝트 전체를 파악하는 데에 있어서 그만큼의 시간이 더 소요된다는 뜻이다. Product는 팀의 코드가 되어야 한다.라는 관점에서 이 코드와 프로젝트는 건강하지 못하다.
“내부가 조금 복잡해도 가져다 쓸 때 편하다.”라는 말은 읽기 어려운 코드에 대한 합리화이다.
이 문장은 늘 생각해야 하면서도, 경계해야 하는 문장이다. 열 줄짜리 코드를 한 줄로 표현할 때 느끼는 희열은 쉽게 빠질 수 있는 함정이다. 조금 극단적으로 표현해보자면 가독성만 놓고 봤을 때 map이나 forEach를 통해 해결하는 것보다 그 자료를 그대로 나열하는 것이 더 좋을수도 있다는 뜻이다.
극강의 간결함을 추구하기 위해 코드를 줄여가는 데에만 초점을 맞추다 보면 가독성 관점을 쉽게 포기한다. 간결하다고 무조건 좋은 코드가 아니다. ‘바보도 이해할 수 있으면서 개발자의 피곤한 중복은 줄일 수 있는’ 중도의 코드에 초점을 맞추어야 한다.
함수의 연산 결과를 항상 값으로만 다루는 것이 아니라 합성으로 다뤄보는 것도 좋은 표현이다.
코드 설명에 대한 예시로 슬랙 메세지 Block을 생성하는 상황을 생각해 보자.
sendSlackForError
) Image Message가 필요하다.슬랙 Message를 생성하는 함수는 createSlackMessage
이다.
맨 처음 작성했던 코드는 이런 식이었다.
const createSlackMessage = (
title: string,
message: string,
imageUrl?: Image,
) => {
const defaultMessage = {...}
if (!imageUrl) return defaultMessage
return {
...defaultMessage,
blocks: defaultMessage.blocks.concat({...})
}
}
slack Message를 생성하는 두 가지 경우가 하나의 함수에서 처리되고 있다. 함수가 한 가지 이상의 일을 하고 있다는 뜻이고, 역할이 분리될 필요가 있다.
여기서 짚고 넘어갈 점은, 역할을 분리하는 것은 성급한 추상화도 아니고 극강의 간결한 표현을 추구하는 것도 아니다. 더 나은 패턴에 대한 고민이다.
ImageBlock을 추가하는 것은 optional이고, 이 상황에는 title과 message가 사용되지 않는다. ImageBlock을 다루는 함수를 분리하고, createSlackMessage
내부에서 삼항연산자로 처리하도록 리팩토링했다.
const createSlackMessage = (
title: string,
message: string,
imageUrl?: Image,
) => {
const defaultMessage = {...}
return imageUrl? addImageBlock(defaultMessage, imageUrl): defaultMessage}
ImageBlock을 concat하는 부분을 분리하니 createSlackMessage
함수의 가독성은 좋아졌다. 하지만 아직 문제가 남았다.
createSlackMessage
함수는 imageUrl
이 필요 없더라도 파라미터를 optional로 명시하고 내부에서 분기문으로 처리하고 있다. 아직 역할의 분리가 완벽히 이루어지지는 않았다는 뜻이다.
필요 없는 정보는 아예 전달하지 않고, createSlackMessage
에서 다루던 ImageBlock에 대한 분기문을 한 단계 상위 함수에서 처리하도록 변경했다.
export const sendSlackForError = async () =>
// ...arg
{
// some variable
return await sendSlackMessage(
TARGET_URL,
addImageBlock(createSlackMessage(ERROR_TITLE, 'message'), imageUrl)
)
}
의식하지 않았을 때는 잘 쓰지 못하던 패턴이다. createSlackMessage
의 값을 별도의 상수에 담아 처리할 수도 있었으나, 이렇게 합성으로 처리하는 것이 더 깔끔하다고 생각했다.
좋은 함수를 작성하는 데에서 어쩌면 가장 중요한 것은, 자료형을 잘 다루는 것이다.
다음 상황을 가정해보자.
convertTemplateString
을 통해 수행된다.generateFiles
의 결과값은 최종적으로 각각 File Write된다.처음에는 하나의 object가 return되도록 작성하였다.
const generateFiles = (fileList, answers) => {
return fileList.reduce((acc, value, index, arr) => {
acc[value] = _this.convertTemplateString(
fn[value](USER_FOLDER_NAME),
answers
)
return acc
}, {})
}
// generateFiles Result
const fileListResult = {
FILE1: 'file Body1',
FILE2: 'file Body2',
}
// fn
const fn = {
FILE1: USER_FOLDER_NAME => {
/* some logic */
},
FILE2: USER_FOLDER_NAME => {
/* some logic */
},
}
fileList
를 다루는 쪽에서는 다음과 같이 작성되어야 한다.
const fileList = generateFiles(['FILE1', 'FILE2'], answers)
Object.keys(fileList).forEach(fileName => {
fs.writeFileSync(fn[fileName](USER_FOLDER_NAME), fileList[fileName], 'utf-8')
})
파악해야 하는 점이 많은 코드다. object를 순회하기 위해 Object.keys
를 이용했고, fn
에서 알맞은 함수를 실행시키기 위해 fn[fileName](USER..)
와 같은 표현이 필요하다.
이렇게 어렵게 작성하지 않아도 된다..
fileList를 object가 아닌 배열로 다룰 경우 Object.keys
없이 바로 순회가 가능하다.
generateFiles
를 처음 작성할 때 자료형에 대한 고민이 부족했고, file을 유동적으로 받을 수 있는 데에만 집중해서 다른 부분을 놓쳤다.
const generateFiles = (fileList, answers) =>
fileList.map(fileName => ({
name: fileName,
body: convertTemplateString(
joinPath(answers[QUESTION_CATEGORY.PROJECT_NAME], fileName),
answers
),
}))
최종 자료형이 name
과 body
를 가진 배열로 완성되도록 변경하였다.
const fileListResult = [
{ name: 'file1', body: 'FILE1' },
{ name: 'file2', body: 'FILE2' },
]
// fn 로직도 joinPath를 사용하는 것으로 변경
fileList.forEach(file => {
fs.writeFileSync(joinPath(projectName, file.name), file.body, 'utf-8')
})
목적에 맞는 자료형으로 변경하는 것만으로도 코드가 훨씬 읽기 쉬워진다. 함수가 return하는 값이 최종적으로 사용되는 곳에서 어떻게 다루어질 것인가를 생각하며 작성해야 한다.
‘좋은 코드’에는 정답이 없다. 이 글에서 다룬 모든 문장은 지금 시점에서 생각한 좋은 코드일 뿐이다. 충분히 다른 답이 있을 수 있고, 나의 답 또한 변할 수 있다.
변하지 않는 답이 있다면, 좋은 코드란 무엇일지 항상 고민하는 사람이 작성한 사람의 코드가 앞으로 더 좋아질 가능성이 높다는 것 정도가 아닐까?