코드 리뷰와 읽기 좋은 코드에 대한 단상 (부제: 동작하는 코드를 넘어서)
우리 팀은 기본적으로 작업건에 대한 PR(Pull Request)을 올리고 리뷰를 받은 후 merge 한다. hotfix 이거나 팀 내 리소스, 일정이 촉박한 경우에는 approve 되지 않은 PR을 merge 하기도 하니 굉장히 빡세다고(?) 할 수는 없지만 그래도 코드 리뷰 시스템을 꽤 잘 운영하고 있다고 생각한다. (10개월 전부터 진행하고 있는 프로젝트 저장소에 지금까지 800개 이상의 PR이 올라왔고, 70% 이상은 리뷰되고 있다.) 최근에 코드 리뷰를 주고 받는 과정에서 ‘읽기 좋은 코드’가 무엇인지 생각해 볼 기회를 얻어, 부족함을 돌아보고 얻은 교훈을 정리해본다.
코드를 작성하고 리뷰하는 방식 되돌아보기
개발자로의 내가
코드를 작성할 때 가장 중요하게 생각하는 것은 ‘기획서의 모든 요구사항을 파악하고, 정확히 동작하도록 구현하는 것’ 이다. 리뷰어로의 내가
PR을 리뷰하는 방식은 [1] 개발할 때와 마찬가지로 기획서의 모든 요구 사항을 파악하고 [2] 코드의 의도를 파악하면서(이 기능을 이렇게 구현했구나) / [2-1] 기능이 정상적으로 돌아가는지 확인하고 / [2-2] 기능과 코드적으로 개선할 부분이 있는지 코멘트를 남긴다. [2-1]이 확인되면 approve 하고, [2-2]는 작성자의 선택으로 남겨둔다.
이렇게 작성하고 보니 무언가 이상한 점이 보인다. 리뷰어로의 나
는 동료 코드의 의도를 파악하기 위해 노력하면서, 작성자로의 나
는 동료가 내 코드를 쉽게 이해할 수 있도록 노력하는 과정이 없다. 물론 의도대로 동작하자마자 개발을 끝내지는 않으며 리팩토링을 진행하지만 그게 내 기준에서 코드를 다듬고 정리하는 수준이지, ‘동료가 읽기 좋은 코드를 만들기 위해’ 라는 목적이 아니였다는 의미이다. 왜그랬을까?
마음 한 켠에 다른 사람이 작성하는 코드를 이해하는 일은 ‘원래’ 힘든 것이기 때문에, 서로가 리뷰에 시간을 들여서 상대의 의도를 파악하려고 노력하는 것이 맞다고 생각했다. 구현하는 방법은 사람마다 다른게 당연한거니까. 큰 문제의식을 가지지 못함으로써 “어떻게 하면 동료가 내 코드 의도를 파악하는데 쓰는 노력을 줄여줄 수 있을까?” 를 고민하지 못한 것이다. ‘원래’ 라는건 ‘원래’ 없다는 것을 잊었다.
또 다른 포인트에서의 방식의 차이도 있었다. 앞서 설명한 것 처럼 나는 기획서를 모두 파악하고 코드를 그에 맞춰서 읽는데, 역으로 코드를 읽으면서 수정되거나 추가된 기능을 파악하는 동료도 있다. 코드에 복잡한 기획이나 도메인 지식이 포함되어 있다면 후자의 방식으로 리뷰를 진행하는 동료가 코드를 파악하는 난이도가 높은건 자연스러운 일이다.
읽기 좋은 코드는 의도를 파악하기 좋은 코드다.
읽기 좋은 코드는 의도를 파악하기 좋은 코드다. 의도를 쉽게 파악하면 코드 리뷰에 소모되는 시간이 단축되며 이는 곧 비즈니스 로직을 더 많이 구현하거나 안정성 있고 확장 가능한 코드를 짜는데 더 많은 시간을 확보할 수 있다는 것을 의미한다. 아니면 다른 가치있는 일에 시간을 쓰거나. 따라서 의도를 파악하기 좋은 코드를 짜기 위해 분명히 노력할 필요가 있다는 것을 느꼈다. 당장은 내가 코드를 작성하는 시간이 더 걸리겠지만 나보다 현재와 미래의 동료, 팀을 위한 것이며 결국 내가 더 나은 개발자가 되는 방법으로 이어진다.
그렇다면 무엇이 의도를 파악하기 좋은 코드일까?
이 부분에 대해서 동료의 피드백과 진유림님의 실무에서 바로 쓰는 Frontend Clean Code 발표에서 많은 힌트를 얻었다. 의도를 파악하기 좋은 코드를 짜려면 먼저 1) 추상화 하기 / 2) 응집도 높이기(역할 분리하기) 두가지 포인트를 신경 쓰자.
추상화 하기
세부 구현은 궁금하면 까보도록! 필요한 만큼만 노출하자
-
로직이 아닌 변수(값, 함수)의 이름만 보고 무슨 역할을 하는지 유추할 수 있는 유의미한 네이밍을 달자
- 예를 들어, 상위 컴포넌트에서 상태 값(state)과 상태 값을 변경하는 함수(setState)를 가지고 있는데, 하위 컴포넌트에서도 상태값을 변경해야 하는 로직이 필요하다면
- props로 state, setState를 그대로 넘기는 대신
- 하위 컴포넌트가 어떤 상황에서 상태값을 변경해야 하는지를 의미하는 네이밍을 가진 함수를 만들고, 로직은 함수 안에 넣고, 그 함수를 넘긴다.
- 핵심 로직을 담고 있는 변수만 노출한다.
- map을 돌아야 하는 부분은 컴포넌트로 분리하여 추상화 레벨을 통일 시킨다.
응집도 높이기 (역할 분리하기)
- 하나의 목적을 가진 코드는 모여있도록 한다.
- 어떤 컴포넌트는 상태값을 가지고 관리하는 로직을 담당하지만, 어떤 컴포넌트는 전달 받은 상태값에 따른 역할을 수행하는 기능만 담당한다.
앞으로
코드 리뷰를 할 때 기능이 기획대로 정상적으로 돌아가는지는 로컬에서 빨리 확인하고 코드적 개선에 좀 더 포커스를 맞춰야겠다. ‘이 코드가 이렇게 되었으면 더 좋았겠다’ 라는 의견을 내는 수준이 네이밍을 넘어 설계의 방식까지 갈 수 있어야 한다. 만약 동료의 코드를 리뷰하면서 읽기가 편했다면 그냥 approve 하지 말고, 어떤 점들 때문에 읽기가 쉬웠는지 탐구하는 것도 큰 도움이 될 것 같다. 잘 읽고 잘 쓰는 능력을 더 기르기. 이번에도 나라는 사람이 뭔가를 배울 때 동기와 경험이 참 중요한 사람이라는 걸 다시 한 번 깨닫는다.