開発言語や開発手法に依存せず、初心者が使えるシンプルで汎用的なコードレビューガイドがみつからなかったので、つくってみました。
1. 集中(60分以内)
集中力の波は15分周期と言われています。
この周期の繰り返しも数回が限界で、60分を超えてレビューし続けてもよいレビューはできません。
60分以内の短い時間で集中してレビューしましょう。
2. リズム
レビューには時間がかかります。
毎朝はじめの60分はレビューにあてるなど、習慣化しましょう。
1回60分以内とすると、多くても500LOCが限界です。
テクニカルリーダーに集中しすぎないように、対象コードの難易度などを考慮して適切にレビュアを割り振ることも重要です。
3. セルフチェック
レビューを依頼する前に、セルフチェックをしましょう。
仕様のとおりに正しく動作することはもちろん、静的コード解析チェックをパスし、自分なりに読みやすく、保守性の高いコードになっているか再確認しましょう。
4. レビューリクエストコメント
レビューを依頼するときに、次のようなレビュアのための補足を伝えましょう。
(プル/マージリクエストのコメントに書くとよいでしょう。)
- 特に確認してほしいところ
- 悩んだところ
- 参照するとよいドキュメント
5. チェックリスト
(1) 網羅性
仕様のとおりに正しく動作することが最重要です。
- コードに明らかなロジックエラーがない
- 要件の観点から、すべてのケースが完全に実装されている
(2) 規約
規約はツールを使って確認しましょう。
自動テスト対象については、テストコードのカバレッジをツールで確認したうえで、内容も含めて十分かどうか判断しましょう。
(自動テストの範囲は事前に決めておきましょう。)
- スタイルチェックや静的コード解析チェックをパスしている
- 適切なコメントがつけられている(コメントをつけるのは美しいコードをかけていない証拠だという人もいますが、それほど我々は優秀でしょうか。)
- テストコードが十分に書かれている
(3) 名前重要
Rubyのまつもとゆきひろさんは、「プログラマが知るべき97のこと」で次のように述べています。
適切な名前をつけられると言うことは、その機能が正しく理解されて、設計されているということで、逆にふさわしい名前がつけられないということは、その機能が果たすべき役割を設計者自身も十分理解できていないということなのではないでしょうか。
パッケージ、クラス、メソッド、変数、これらのすべてが簡潔かつ適切に内容を言い表しているのであれば、オブジェクト指向、DDD(ドメイン駆動設計)のよいコードになっていると思ってよいでしょう。
(4) DRY
"Don't Repeat Yourself"の略で、要はコピペ禁止ということです。
コピペコードがあると、変更があったときに修正対象が散在しているので困ります(保守性が低い)。
DRYはチーム開発のマナーです。
(5) 単一責務
1つのクラスは1つの責務を持つという原則ですが、「 クラスに変更が起こる理由は、一つであるべき」という具体的な指針がわかりやすいでしょう。
もし、変更の理由が2つあ る場合、そのクラスは2つに分割すべきだし、もし1つの変更が2つのクラス にまたがるなら、そのクラスは1つに統合すべきです。
(6) 疎結合かつ高凝集
疎結合は、プログラム間の依存性を弱め、再利用性を高めるということです。
高凝集は、同じ責務のものが集まっていて分散していないということです。
MVCなどのレイヤー分けをしているのであれば、最低限、レイヤー間は疎結合にしましょう。
クラスやメソッド間もできるだけ疎結合になるように注意しましょう。
呼び出し側が呼び出し先の仕様を知らなくてもよいのが理想です。
(7) シンプル
シンプルとは、手続き型で一直線に書くという意味ではありません。また、高度なテクニックを駆使して短いコードにするという意味でもありません。
標準の仕様、標準のAPIを適切に使って、簡潔でわかりやすいということです。
保守性は大事ですが、過度に柔軟な設計は不要です(YAGNI ”You Aren’t Going to Need It"(わからないことを予想して複雑にしない))。
デザインパターンや流行りの技術を「ただ使いたいから」という理由で使ってしまうのもよくありません(モチベーション維持のために多少は必要かもしれませんが)。
6. 完璧を求めない
我々に与えられた時間は有限です。
美しいコードにしたいという気持ちは重要ですが、仕様通りに正しく動くものを短い時間でつくりあげることが、仕事では求められます。
後になって、運用・保守の担当者に「なんだ、この××コード」と言われないように、エンジニアとしてのプライドが許すレベルに、健康で文化的な生活ができる範囲で、スピード感をもって、コードを美しくしましょう。
7. 問題vsわたしたち(批判ではない)
今さらですが、レビューは批判ではありません。よりよいものをつくりだすためのプロセスです。このことはいくら強調しても協調しすぎることはありません。
レビュアは客観的に理由を明示して指摘しましょう。
レビュイは問題の事実だけを受け止め、落ち込んだり、バカにされていると思わないようにしましょう(昔から「傍目八目」というものです)。
問題vs誰かではなく、問題vsわたしたちの姿勢でレビューしましょう。