백세코딩#코드리뷰

코드리뷰

개발자에게 코드 리뷰가 어려운 이유

1. 리뷰는 언제나 상호 합의가 된 상황에서 진행되어야 한다.

2. 리뷰어의 해당 결과물에 대해서 객관성을 가지고 서로 인지해야 한다

3. 개발자 자신의 작업물에 대해서 정말 객관적으로 바라볼 수 있는 작성자가 선정되어야 한다.

소프트웨어 코드는 정량적인 검토와 정성적인 검토를 구분해야 한다.

구분이 모호해지면 리뷰는 그 방향성을 상실하게 된다.

주목받는 개발 방법론의 추세는 '테스팅'을 주로 하고 있다. SRS와 같은 요구사항에 집중하기 보다는 TDD와 같은 방법으로 완성된 산출물의 품질을 높이는 방법을 주로 사용하고 있다.

당장의 성과물을 위해서라면 코드 리뷰보다는 테스팅에 집중하는 것이 더 효율적이다. 빠르게 고속개발하고 테스트를 통해서 버그를 찾은 다음 수정하는 것이 '특정 기능들을 나열하고 기능을 만족하는 소프트웨어'의 경우에는 테스트 주도 개발방법이 가장 적합하다고 할 수 있다.

SRS중심이건, TDD중심이건 코드리뷰는 중요하다.

코드리뷰는 기능나열이 아니라 어느정도 이상의 복잡도나 코드 품질이 필요한 경우에 수행하는 것이 더 현명하다.

일반적인 SI의 형태라면 워크스루의 형태만 적합하다.

특정 도메인에 매몰되어 있고 처리방법도 명쾌하기 때문에 해당 경험들을 교환하는 것으로도 충분하기 때문이다.

그리고 자동화된 테스트 수행방법을 최대한 갖추어두는 것이 현명하다.

그러므로 코드 리뷰는 어느정도 솔루션이나 서비스 등을 고려하는 곳에서 더욱 적합하다고 이야기 하겠다.

그리고 코드리뷰는 특정 제품이나 서비스를 발전적으로 지향하는 경우라면 필수적으로 선택해야 한다.

하지만 일부제품은 발전적인 지향이 굳이 필요없는 제품군을 가진 경우도 많다.

그 경우에는 선택적인 코드리뷰를 지향하면된다.

비용상의 문제 때문에 굳이 코드리뷰를 억지로 진행할 필요없는 경우도 많다.

대부분의 소프트웨어 개발은 테스트 케이스를 잘 만들고 통과시키는 것으로써 충분한 신뢰를 가지면 충분한 경우가 대부분이다.

코드리뷰가 필요없는 경우 체크리스트

1. 특정 도메인만 다루는 팀이나 회사의 개발팀인가?

2. 지난 2~3년 정도 솔루션이 크게 변한것이 없으며 향후에도 기업이나 팀에서 투자가 없을 예정이다.

3. 현재 개발자들이 해당 솔루션이나 개발일을 5년 이상하고 있다.

4. 기능위주의 SI성 업무를 주로 처리하고 있으며, 복잡한 알고리즘은 존재하지 않는다.

5. 비용과 일정상 개발팀에게 리소스 투여가 불가능하다.

1개이상이라도 체크된다면 코드리뷰는 성립하기 어렵다.

TDD나 테스트 케이스를 가능한 많이 축적하여 소프트웨어 품질을 올리기를 권장한다.

코드 리뷰가 필요한 경우 체크리스트

1. 다국어와 시장이 다변화된 환경에서 소프트웨어 구동되어야 한다.

2. 코드의 복잡도가 높으며 단순 기능 나열의 요구사항이 아니라 소프트웨어 아키텍처가 별도로 구성되기 시작하였다.

3. 사용자의 경험성을 중가하기 위하여 매우 많은 변화가 예측된다.

4. 현재 개발중인 서비스는 중단없이 지속적으로 반전하여야 하는 서비스이다.

5. 목표 요구사항이 계속 변화하고 있고 프레임워크를 지향하여 소프트웨어 품질의 요구사항이 매우 중요하다.

하나라도 해당되면 코드리뷰는 매우 효과적으로 소프트웨어에 의미 있는결과물을 얻어내기 위한 좋은 방법이 된다.

코드리뷰의 정도와 질에 대한 검토

1. 실험적인 코드인가?

2. 1~2명이상 공동으로 작업하는 코드인가?

3. 앞으로 버려질 가능성이 큰 코드인가?

코드리뷰를 하지 않으면 해당 코드의 리포지터리나 디렉토리를 완전하게 분리하고 리뷰가 안된 코드를 명쾌하게 구분할 수 있어야 한다.

그리고 그 정보는 팀 전체에게 공개되어야 한다.

코드리뷰 가이드라인

1. 코드 검토는 1시간 이내에 끝낼 분량으로 검토한다.

2. 코드는 200라인 이상을 한번에 검토하지 마라.

코드리뷰를 하는동안 기능에 대한 검토 체크사항

1.시스템의 요구사항이 제대로 반영되었는가?

2. 시스템 설계 규격대로 구현되었는가?

3. 과도한 코딩을 하고 있지 않는가?

4. 같은 기능 구현을 더 단순하게 할 수 있는가?

5. 함수의 입출력 값은 명확한가?

6. 빌딩블록들(알고리즘, 자료구조, 데이터 타입, 템플릿, 라이브러리, API 등)이 적절하게 사용되었는가?

7. 좋은 패턴과 추상화(상태도, 모듈화 등)을 사용해서 구현하고 있는가?

8. 의존도가 높은 함수나 라이브러리 등의 의존관계에 대해서 별도 기술하고 있는가?

9. 함수의 반환(exit)은 한곳에서 이루어지고 있는가?

10. 모든 변수는 사용 전에 초기화하고 있는가?

11. 사용하지 않는 변수가 있는가?

12. 하나의 함수는 하나의 기능만 수행하고 있는가?

스타일과 코딩가이드

1. 코딩스타일 가이드를 준수하고 있는가?

2. 각 파일의 헤더 정보가 존재하는가?

3. 각 함수의 정보를 코드에 대해서 설명하기에 충분한가?

4. 주석은 적절하게 기술되어 있는가?

5. 코드는 잘 구조화(가독성, 기능적 측면)되어 있는가?

6. 헤더, 함수 정보를 도구로 추출해서 자동으로 문서화할 수 있는 구조인가?

7.변수와 함수의 이름이 일관되게 기술되어 있는가?

8. 가이드를 통한 네이밍 규칙을 준수하고 있는가?

9. 숫자는 단위에 대해서 기술하고 있는가?

10. 숫자를 직접 서술하지 않고 상수를 사용하고 있는가?

11.어셈블리 코드를 사용하였다면 이를 대체할 방법은 없는가?

12. 수행되지 않는 코드가 없는가?

13. 주석 처리된 코드는 삭제(버전 체크)되었는가?

14. 간결하지만 너무 특이한 코드가 존재하는가?

15. 설명을 보거나 작성자에게 물어봐야만 이해가 가능한 코드가 있는가?

16. 구현 예정인 기능이 있다면, ToDo 주석으로 표시되어 있는가?

아키텍처에 검토

1. 함수의 길이는 적당(화면을 넘기면 안된다)한가?

2. 이 코드는 재사용이 가능한가?

3.  전역변수는 최소로 사용하였는가?

4. 변수의 범위는 적절하게 선언되었는가?

5. 클래스와 함수가 관련된 기능끼리 그룹화되었는가?

6. 관련된 함수들이 흩어져 있지 않는가?

7. 중복된 함수나 클래스가 있지 않는가?

8. 코드가 이식성을 고려하여 작성되었는가?(프로세스의 특성을 받은 변수타입이 고려되어 있는가?)

9. 데이터에 맞게 타입이 구체적으로 선언되었는가?

10. if/else 구분이 2단계 이상 중첩되었다면 이를 함수로 더 구분하라.

11. switch/case 문이 중첩되었다면 이를 더 구분하라.

12. 리소스에 lock이 있다면 unlock은 반드시 이루어지는가?

13. 힙메모리 할당과 해제는 항상 짝을 이루는가?

14. 스택변수를 반환하고 있는가?

15. 외부/공개 라이브러리 사용하였을 때 MIT 라이선스를 확인했는가?

- GPL 경우에 관련된 영역에서만 사용해야 한다.

16. 블락킹 api 호출시에 비동기적인 방식으로 처리하고 있는가?

예외처리 관련 체크

1. 입력 매개변수의 유효범위는 체크하고 있는가?

2. 에러코드와 예외(exception)의 호출함수는 분명하게 반환되고 있는가?

3. 호출함수가 에러와 예외처리 코드를 가지고 있는가?

4. null 포인트와 음수가 처리되는 구조인가?

5. 에러코드에 대해서 명쾌하게 선언하고 처리하고 있는가?

6. switch 문에 default가 존재하고 얘외처리를 하고 있는가?

7. 배열 사용시에 index 범위를 체크하는가?

8. 포인트 사용시에 유효한 범위를 체크하는가?

9.Garbege collection을 제대로 하고 있는가?

10. 수학 계산시 overflow, underflow가 발생할 가능성이 있는가?

11. 에러 조건에 체크되고 에러발생시 로깅 정보를 남기는가?

12. 에러 메시지와 에러코드가 에러의 의미를 잘 전달하는가?

13. try/catch 에러 핸들링 사용방법은 적절하게 구현되었는가?

요즘 프로그램은 대부분 이벤트성으로 구동되지만, 시간의 흐름에 대한 체크는 프로그램의 뼈대를 이루게 된다.

검토

1. 최악의 조건에 대해서 고려하였는가?

2. 무한루프와 재귀함수는 특이사항이 아니라면 없어야한다.

3. 재귀함수 사용시에 call stack 값의 최대값이 고정되어 있는가?

4. 경쟁조건이 존재하는가?

5. 스레드는 정상 생성, 정상 동작하는 코드를 가지고 있는가?

6. 불필요한 최적화를 통해서 코드 가독성을 희생하였는가?

7. 임베디드도 최적화가 매우 중요하지 않다면 가독성을 더 중요하게 해야 한다.

검증과 시험에 대해서인지

테스트를 위해서 가능한 최대한 자동화를 하기 위한 방법들을 이용한다.

1. 코드는 시험하기 쉽게 작성되었는가?

2. 단위 테스트가 쉽게 될수 있는가?

3. 에러 핸들링 코드도 잘 테스트 되었는가?

4. 컴파일, 링크 체크 시에 경고 메시지도 100% 처리하였는가?

5. 경계값, 음수값, 0/1 등의 가독성이 떨어지는 코드에 대해서 충분하게 경계하고 있는가?

6. 테스트를 위한 fault 조건 재현을 쉽게 할 수 있는가?

7. 모든 인터페이스와 모든 예외조건에 대해서 테스트 코드가 있는가?

8. 최악의 조건에서도 리소스 사용은 문제 없는가?

9. 런타임시의 오류와 로그에 대비한 시스템이 있는가?

10. 테스트를 위한 주석 코드가 존재하는가?

하드웨어에 대한 테스트

1. I/O  오퍼레이션 코드에 대한 테스트로 하드웨어가 정상적인 동작을 보장하는가?

2. 최소/최대 타이밍 요구사항에 대해서도 하드웨어 인터페이스가 충족하는가?

3. 멀티바이트 하드웨어 레지스터가 read/write 오퍼레이션 중에도 값이 바뀌지 않음을 보장하는가?

4. 시스템이 잘 정의된 하드웨어 상태로 리셋하는것을 S/W가 보장하는가?

5. 하드웨어의 전압이 떨어지거나 전원이 차단되는 경우 잘 처리하는가?

6. 대기모드 진입시와 빠져나올때에 시스템에 옳게 동작하는가?

7. 사용하지 않는 인터럽트 백터가 에러 핸들러에 연결되어 있는가?

8. EEPROM손상(데이터 깨짐)을 막기 위한 매커니즘이 있는가?(쓰기 동작중 power loss 등 )

코드리뷰에 대한 기업과 적당한 방법

코드 인스팩션(inspection)은 가장 정형화된 기법으로 전문화된 코드 리뷰팀을 통해서 구분하는 방법이다.

인스택션을 하기 위한 롤

Moderator: 실질적인 매니저로 팀 간의 인터페이스와 리소스, 인프라를 확보하고 프로세스에 대한 정의와 산출물의 정리를 담당한다

Reader: 각 산출물을 읽고, 리뷰하고, 방향성을 제시한다. 보통, 지식이 많은 사람이 담당한다

Designer/Coder: Reader의 지시에 따라서 코드를 검증하고 잠재적인 발견 등의 수정방안을 만든다.

Tester: 진행중인 코드와 권장 수정 코드에 대해서 검증한다.

인스팩션 6단계

1. planning: 계획수립

2. Overview: 교육과 역할정의

3. Preparation: 인터뷰와 필요한 문서 습득, 툴 환경 구축

4. Meeting(Inspection): 각자의 역할대로 수행

5. Rework: 보고된 Defect 수정

6. Follow-up :보고된 Defect 가 수정되었는지 확인

코드 인스팩션이 수행되면 상당히 명쾌한 리뷰가 진행된다.

비용문제때문에 스타트업에서 선택하기 어렵다.

그래서 사용하는 방법 중 하나가 팀리뷰이다.

팀 리뷰는 일정한 계획과 프로세스만  따르는 방법으로 코드 인스팩션보다는 덜 정형화된 방법으로 진행한다.

일주일에 한번정도 팀리뷰를 수행하거나 특정 모듈이나 기능이 완료되는 시점을 기준으로 테스트 결과를 가지고 리뷰를 하는방법을 사용한다.

또한 위험하거나 의견이 필요한 경우에도 팀 리뷰는 유용하다.

일반적인 팀에서 사용하는 방법이다.

하지만 리뷰에 대한 제대로 된 인식이 없다면 적용하기 어렵다.

그래서 가끔 사용되는 방법이고 과거 국내 SI 업체들이 주로 사용하던 방법중 하나가 워크 스루이다.

이 방법은 특정 도멘인에 종속된 코드를 만들거나 비슷한 SI형태의 업무를 수행하는 경우에 적합하다.

그래서 국내 SI업체에서 적극적으로 사용되면 좋겠지만 이 시간마저도 부정확하고 갑을병정의 SI 체계에서 정보공유나 아이디어 수집과 같은 커뮤니케이션이 자유롭게 일어나는 것은 매우 힘들다.

이 워크스루는 동일한 조직 내에서 동일한 목적의식이 분명한 팀에서나 활용할 수 있는 방법이다.

국내 스타트업이나 IT전문기업들은 '리뷰'에 대해서 상급관리자들이 제대로 허락을 해주지 않는다.

대부분은 팀 내에서 어떻게든 자체적으로 해보려고 한다.

그래서 팀장의 권한 선에서 적절하게 리뷰를 하는 방법 중이 하나가 Peer review or over the shoulder review 방법이다.

이 방법은 보통 2~3명이 진행하는 코드 리뷰로 코드의 작성자가 모니터를 보면서 코드를 설명하고, 다른 한사람이 설명을 들으면서 아이디어를 제안하거나 결함(Defect)을 발견하는 방법이다.

또한 이 방법은 신입사원이나 인턴사원의 경우 업무 이해도를 높이면서 해당 코드를 사용할 수 있는 수준으로 활용할 때의 의미있는 방법이다.

문제는 이 방법이 개발자 투입이 거의 두 배 이상으로 증가하는 것으로써 고품질의 영역을 개발하거나 빠른 시간 안에 신입 개발자의 업무 이해도를 높이는 경우가 아니라면 시행하지 않는다.

이렇게 리뷰가 진행이 되지 않으면 돌려보기 방법(Passaround)을 사용한다.

상세한 리뷰는 아니다. 온라인이나 실시간성이 아니라 리파지토리나 이메일 등을 사용하여 천천히 리뷰하는 방식에 해당하는데 속도는 느리지만, 중요한 코드이거나 제품의 기능개선이 필요한 경우에는 아주 의미가 있다.

보통은 제품의 기능개선을 위해 사용하는 방법이다.

리뷰에 다양한 방법은 있지만 결론적으로 어느정도 개발조직이 서로 커뮤니케이션 하고 목적의식을 통일하여 적절한 시간 분배를 통해서 리뷰를 할 수 있는 시간을 만들어 내느냐가 리뷰의 핵심이라고 할 수 있다.

리뷰를 통해서 소프트웨어의 품질을 끌어올리고 개발자들과 소통하여 방향성을 만들어 내며 새로운 기능 개선작업을 위해 리뷰는 다양하게 활용된다.

어떤 관점으로 리뷰를 할 것이고 어떤 관점으로 리뷰라는 프로세스를 개발 프로세스에 탑재할 것인가에 대해서 진자하게 고민하는 것 그것이 아키텍트의 첫번째 역할 아닌가 한다.

출처-백세코딩