코드 리뷰가 개발 문화에 미치는 영향

코드 리뷰를 하기전 개발 프로세스

- 이슈 기반의 개발을 전혀 하지 않고 있지 않음

: 구두, 메신저, 메일, 위키 등으로 업무가 할당됨

 

- 코드 또는 설계에 대한 리뷰 과정이 없음

: 서비스간 인터페이스만 정의하고 인터페이스 스펙만 만족

 

- 형상관리 도구로 git을 사용하고 있지만 개발자가 직접 upstream으로 push하고 있음

 

- 소스코드의 품질관리가 안되고 있음

: 소스코드 내 중복코드와 변수명, 모듈화에 대한 고민 전무

 

- 신규로 개발해야하는기능이 훨씬 많은 상황

 

 

 

 

 

코드 리뷰 도입을 통한 개발 과정 개선

- 코드 리뷰를 위한 Pull Request 도입

 

- Fork를 통한 개인 레포지터리 생성 후 작업

 

- 모든 Pull Request는 이슈 기반으로 생성

 

- Commit 로그에는 반드시 이슈번호를 기재

 

- 코드 리뷰에 작성된 의견이 모두 해결되기 전까지는 Merge 불가

 

- 코드 리뷰에 이견이 있으면 이견을 추가하고 토론

: 코드 리뷰어의 내용이 반드시 옳은 것이 아니며, 그 내용은 지시가 아닌 의견

 

- 리뷰에 등록된 모든 사항에 대해 확인이 되어야 Merge

 

 

 

 

 

 

좋은 코드 리뷰 사례

- 코딩 컨벤션은 잘 지키고 있는가?

 

- 코드가 이해하기 쉬운가?

 

- 변수명 또는 함수명은 이해하기 쉬운가?

: 해석하기 어려운 약어 또는 두 글자의 변수 사용

: 변수명이 해당 변수가 가지고 있는 값과 동일하게 사용되었는지 확인

: 변수명에 1이나 2 사용 여부

 

- 의미가 애매모호한 상수의 사용

 

- 사용하지 않는 주석 처리된 코드 삭제

 

- Logger를 사용하는가?

: 상황에 맞게 로그 레벨을 사용하는지 확인

 

- 코드가 중복되지는 않는가?

 

- 함수가 너무 길지 않는가?

: 너무 긴 경우 코드를 읽기 어렵고 테스트 코드 작성이 어렵기 때문

 

- 너무 깊은 레벨의 if문

: 3단계 이상의 if문은 지양

 

- if, for, while 문에서 조건에 대한 처리

: 조건문이 너무 길다면 함수화를 권장

 

- Enhanced for loop (for-each) 사용 권장

: for loop 작성 시 index를 이용하여 반복 조건을 명시하는 것보다 enhanced for loop 사용을 권장

 

- 테스트 코드가 있는 경우 테스트 코드 부터 확인

 

- 테스트 코드도 읽기 쉽고 중복 등이 없는지 확인

 

- 기존 코드에 대한 수정인 경우 리팩토링을 할수 있는 부분이 없는지 확인

 

- 데이터베이스 처리와 관련된 경우

: UPDATE, DELETE 문에 WHERE 조건이 있는지 확인

: WHERE 조건이 있는 코드에서 DB에 Index가 잡혀있는지 확인

: 트렌잭션 처리가 되는지 확인

: 일련번호를 생성하는 로직에서 동시에 요청을 받는 경우 중복이 발생하는지 확인

: 하나의 SQL에 서로 다른 여러 SQL문을 실행하는 경우

 

- 시간 관련 처리

: UTC 기준인지, 로컬 시간 기준인지

 

- 예외 상황에 대한 처리가 잘 되어 있는지

 

- 동시 접속 시 문제되는 상황은 없는가?

 

- 소스코드 내 보안 관련 문자열이 있는지 확인

: DB 패스워드, 외부 API의 보안 토큰, JWT 토큰 복호화 키

 

- 성능에 영향을 미치는 코드는 없는가?

: 코드에 sleep이 있는 경우

: 데이터가 많은 상황에서 중첩된 For Loop 이용

: Page 처리 없이 모든 데이터를 로딩

: for, while이 무한 반복될 수 있는 경우

 

 

 

 

 

 

Reference

코드 리뷰 이야기(1) | Popit

코드 리뷰 이야기(2) | Popit

가능성을 가치로! 위시켓 · 위시켓(Wishket)