[퍼옴]코드 리뷰 요청 전 셀프 체크리스트

프로그래머 경력 20년이 내가 생각한 코드 리뷰 요청 전 셀프 체크리스트

 

출처: https://qiita.com/y-some/items/d9bd43d683e429fb3afb

 

 

체크리스트

• 맞춤법 오류가 없나?
• 죽은 코드나 의미 없는 주석이 있는 것은 아닌가?
• 주석이 코드와 다르지 않나??
• 복사한 부분을 검토 했나?
• 들여 쓰기가 어긋나지 않았나?
• 컨텍스트와 이름이 일치하는가?
• 타입이 너무 크거나 함수가 너무 크지 않나?
• 한 줄이 (옆으로) 너무 길지 않나?
• IDE에 경고가 발생하지 않았나?
• 수평 확장 조사 했나?
• 기존의 공통 처리가 있다는 것을 놓치지 않았나?

 

 

 

각 항목의 이유와 배경 등

 

맞춤법 오류가 없나?

• 케어리스 미스의 대표적인 것이다.
• 철자 오류가 치명적인 버그의 직접적인 원인이 되는 것도 드물지 않지만 나중에 코드를 검색 할 때 찾기 힘들다는 것도 위험하다.
• 영어 단어의 철자는 조금이라도 확실하지 않으면 구글링으로 확인한다.

 

 

죽은 코드나 의미없는 주석이 남아 있지 않나?

• 「시행착오 단계에서 쓴 코드가 그대로 리뷰 때까지 남아 버렸다」라고 하는 것이, 일반적인 패턴이다.
• 나중에 코드를 읽을 때 시행 착오 코드의 의도를 읽을 수 없다. 나중에 누군가에게 "삭제해야 할 것 같지만, 삭제해서 버그가 생긴다면 싫다 ..."라는 불필요한 수고를 만든다.
• 또 위의 항목과 반대로, 나중에 코드를 검색할 때 검색이 걸려서 귀찮게 한다.
• 나는 시행 착오 용으로 쓰는 코드에 미리, TODO: 시행착오용 코드 같은 나름대로의 마크를 결정하고, 코드 리뷰를 의뢰하기 전에 검색한다.

 

 

코멘트가 코드와 다르지 않나?

• 흔한 패턴은, 「복사 붙여넣기 했을 때에, 주석과 코드를 원본에서 복사해 와서, 이 주석이 복사된 곳과는 매치하고 있지 않다」라는 경우이다.
• 이것도 나중에 누군가에게 "주석 잘못처럼 보이지만, 실은 뭔가 뒷사정이 있지 않을까?…"라는 불필요한 수고를 만든다.

 

 

복사한 부분을 검토 했나?

• 구현의 의도를 누군가에게 물었을 때 「아, 그것은 ○○로부터 복사했을 뿐입니다(그러니까 따지지 말아 주세요)」라고 대답하고 있지 않나?

• 첫째, 논리가 공통이면 공통화를 고려해야 한다 .2
• 둘째, 복사 소스 코드가 테스트 되고 잘 작동하더라도 붙여 넣기에서 예상대로 작동한다는 보장은 없으므로 충분히 확인해야 한다.

• 이러한 copipe를 용서하는 문화가되면, 이 코드는 또한 누군가에 의해 깊이 생각하지 않고 복사된다. 이러면 저장소가 혼돈으로 향한다.
• 그래서 복사-붙여넣기에 트랩이 숨어있는 경우가 많기 때문에 copipe한 부분은 면밀히 검토해야 한다.

 

 

들여 쓰기가 어긋나지 않나?

• 코드의 체제는 명명과 마찬가지로 나중에 코드의 의도를 이해하는 속도에 영향을 미친다.
• 이것은 변경 차이만으로는 눈치 채지 않는 경우도 있으므로 요주의이다.
• 이것 역시, 복사-붙여넣기 원인이 경우가 많다. 복사-붙여넣기의 원래와 앞에서의 중첩 깊이가 다른 경우에 어긋나 버린다.
• 대부분의 IDE에는 자동 정형 기능이 붙어 있으므로, 자동 정형의 바로 가기를 손가락에 기억시켜 두는 것이 좋다 .

 

 

컨텍스트와 이름이 일치하는가?

• 클래스의 이름과 책임, 함수의 이름과 처리 내용, 변수의 이름과 용도, 이다.
• 나중에 누군가가 코드를 읽을 때 이름에서 많은 정보를 얻는다. 프로그램은 쓰기 시간보다 읽기 시간이 훨씬 많기 때문에 명명은 생산성과 품질에 큰 영향을 미친다.

 

 

타입이 너무 크거나, 함수가 너무 길지는 않나?

• 이 근처는 가능하면, Linter 라이브러리를 넣어 기계적으로 체크해야 한다.
• 기준으로는 아래와 같다

• 클래스나 구조체의 행수는 200행 이내
• 함수의 행 수는 40행 이내

• 이 문제의 주제는 행 수가 길어 이해가 어려워지는 것뿐만 아니라, 본질적으로는 「책무가 너무 클 가능성이 있으므로 분할을 검토하자」 라고 하는 것이다.
• 이 관점에서는 혼자 고민하는 것보다 빨리 제삼자의 지혜를 빌리는 것도 의미 있다고 생각한다.

 

 

한 줄이 (옆으로) 너무 길지 않나?

• 기준으로는 120자 이내입니다.
• 프레임워크 등에 의해 자동 생성된 코드도 긴 경우도 있으므로, 거기까지 중요성이 높은 것은 아니지만, 역시 코드의 독자에게 배려하는 차원에서 중요하다고 생각한다.

 

 

IDE에서 경고가 발생하지 않았나?

• Linter를 넣고 있어도, Linter에 의한 경고를 무시해 버리면 엉망이다.
• 이른바 ‘갈라진 창 이론’이라는 녀석으로 ‘이 경미한 경고 정도는 …  라고 그냥 넘겨버리면 바로 경고 수가 수십 수백이 되고, 정말 중요하고 대처해야할 경고를 보지 못할 수 있다.
• Linter 이외에도, 언어나 IDE에 의해, 미 사용 변수나, 메모리 누수, 여러가지 경고를 내 주고 있으므로 기본적으로는 모두 대처해야한다고 생각한다.

 

 

수평 확장 조사 했나?

• 특히 신규가 아니고 개수의 경우, 수정해야 할 부분이 그 밖에 없는지 프로젝트 소스 전체에 대해 검색을 걸어 조사하는 것이 좋다.
• 손을 넣은 「없다」 개소가 문제가 없다고 하는 시점은 실무적으로 소중 하기도 하다.

 

 

기존 공통 처리가 있다는 것을 놓치지 않았나?

• 이쪽도 위의 항목과 유사하고, 「쓴 코드는 올바르지만, 원래 코드를 쓸 필요가 없을 가능성은 없는가?」 라고 하는 시점이다.
• 공통 처리가 벌써 있는데 자신이 구현을 늘려 버리면, 장래의 사양 변경 시에 횡전개 조사·개수가 필요하게 되어 버리고, 놓치는 위험성도 있다.