はじめに
最近全員レビューをチームに導入しました。全員レビューを開始して1ヶ月くらい経ったので、今後のコードの品質向上に役立てる目的で、直近のレビューの指摘点を大雑把にカテゴライズしてみました。チームの構成としては、超ベテラン1名、ノーマル1名、ルーキー2名、ひよこ1名のメンバー構成となっています。チーム発足以来、ずっとペアモブでのプログラミングを進めてきたので、レビューに関しては、ほぼほぼ初心者の集まりです。
環境
- GitHubのプルリク
- Googleスプレッドシート
- 手作業で頑張る
結果
今回の指摘点を大雑把にカテゴリ分けしてみたところ、次の4つのカテゴリに分けられました
- コードの可読性に関すること（コメント、変数の命名などについて）
- コードの保守性に関すること（クラス設計やDRYなコードなどについて）
- 無駄コード（デバッグログの残り、コードの消し忘れなど）
- 機能実装漏れの指摘
割合はこんな感じです。
| カテゴリ | 可読性 | 保守性 | 無駄 | 実装漏れ | バグ |
|---|---|---|---|---|---|
| ％ | 46.8 | 32.3 | 12.9 | 6.5 | 1.6 |
圧倒的に可読性に関することが多いですね。
各カテゴリについて
コードの可読性に関すること
主にコメントについての指摘が一番多かったです。
コメントについて
- 3行コメントを付け忘れている
私のチームでは、メソッドの頭に3行コメントをいれるように決めているのでそのつけ忘れが多い - HowではなくWhy
コメントを書く際に、コードに書いていることをそのまま日本語に翻訳しただけコメント多い、「なんのための処理」なのか「意図はなんなのか」などをコメントに書いてほしいと指摘している - 書いている本人はわかっているが、レビューする人には伝わってない
コードを書いている際は、当然何をしているコードなのか理解できているので、特にコメントを必要に感じないが、レビューを通してコードを書いてない人が見ると、理解するのに時間がかかるので読みやすいようにコメントをつけてほしいと指摘している。 - 消し忘れ、不適切、修正漏れ
メソッドの名称と内容を変えたのに、コメントは変更し忘れていることの指摘
命名
- メソッドは動詞
メソッドの始まりは動詞になるように決めています - 真偽値を持つ変数はis~
真偽値（True/False）は「is〜」のように真か偽で答えられる形式にして読みやすくしています。 - (余談)codic
同じ動作を表す単語が複数あり、チーム内で意見が割れる際にはcodicというサービスを使用しています
コードの保守性に関すること
- クラス設計に関すること
処理が一箇所にまとまりすぎているので、新しくクラスに切り分けたい
他のクラスに置くべきではないかという責任範囲の話 - DRY
既存の処理をコピペして目的の動作を実現した後、共通化することなくそのままにしていることに対する指摘
既存の定数やメソッドに気づいてなくて使用していないことの指摘
独自の書き方ではなくすでに他の処理で実装されている書き方に統一すべきという指摘
無駄コード＆機能実装漏れの指摘
特に記載することはないです。
やってみて
レビューを本格的に始めてみて、1レビューに対する指摘コメントが多いことが問題にあがり、指摘点を減らす目的のために、この取り組みをやってみました。この取り組みの中で、どんな指摘点があるのかチーム内で共有、再認識できることができました。認識することで、レビューに慣れてきたこともありますが、この取り組みの翌週からケアレスミス的な指摘の数はどんどん減っていきました。やらないよりは、チームのコードに対する意識の成長につながったのではと思っています。