[20.12.24] 객관적인 코드 리뷰를 위해

Reference

johnnydev 유튜브

Why

지금까지 다양한 이유들로 프로그래밍을 하고 있지만, 내가 짠 코드에 대해 확신이 서지 않을 때가 지금도 종종 있다. 그 막연함을 글로써 정리해보면 내가 가진 문제점을 파악할 수 있을 거라는 생각에 johnnydev 님의 영상 내 정보를 내 블로그에 옮겨 보기로 했다.

정성적 vs 정량적 분석

• 정성적 근거 : 응답자의 주관적인 견해를 토대로 분석하는 방법
• 정량적 근거 : 구체적인 수치를 토대로 분석하는 방법

=> 코드 리뷰는 서로를 위해 가능한 한 정량적일 필요가 있다.

정성적일 수 있는 근거

1. 파라미터 개수

• 메서드 하나가 여러 동작을 수행하고 있는 것일 수 있다.
• 반대로 여러번 쓰이지 않을 함수임에도 불구하고 쪼개는 것에만 치중해 메서드를 나누는 것일 수 있다.

2. 중첩되는 if문

• 주요 로직 실행 전에 예외처리 및 반환 가능한 상황을 미리 처리해준다.
  => 나는 이러한 방식을 Early return 이라 부르는 줄 알았는데 Guard Clause 라고도 하나 보다.

3. 순수 함수

• 동일 입력 값에 동일 출력을 하는 함수
• 외부 값을 수정하지 않음
• 인자를 수정하지 않음

비순수 함수의 경우 로직이 처리되는 단계에 따라 찾고자 하는 값이 달라지기 때문에 디버깅이 힘들어진다는 특징이 있다. (e.g. 전역상태의 state) 하지만 그렇다고 비순수 함수를 아예 제거할 수는 없기 때문에 순수 / 비순수 함수의 영역을 최대한 명확하게 나누려는 노력이 필요하다.

정량적일 수 있는 근거

1. 순환 복잡도

• 소스 코드의 복잡도를 나타내는 지표
• eslint 의 rule 로도 존재한다. ("complexity")

위의 수치로만 판단했을 때는 간단한 문과 복잡한 문 사이의 인지 복잡도, 즉 실제로 이해하기 얼마나 어려운지는 파악하기 힘들다고 한다. 그래서 보안책으로 나온 것이 cognitive-complexity rule이고 현재 eslint에서 지원하고 있다고 한다. 나중에 기회가 되면 써봐야겠다.

2. Branch Coverage

• 코드의 분기에 따른 모든 실행 경로를 테스트 했는지 나타내는 지표
• 함수에 if 가 하나라면 경로는 2개지만, 중첩될수록 경로는 제곱으로 증가한다.
• istanbuljs 를 통해서 측정 가능하다.

느낀 점

개인적으로 정량적인 근거 파트는 TDD 방식에도 큰 도움이 될 거라고 생각한다. 아직까지는 테스트 주도 개발에 대한 능력이 많이 부족한 터라 영상에 나오는 정보를 모두 이해하지는 못했지만, 차근히 알아가야겠다.