『伝わるコードレビュー 開発チームの生産性を高める「上手な伝え方」の教科書』を読みました！ ~心構え編 第2弾~
こんにちは！今回も『伝わるコードレビュー 開発チームの生産性を高める「上手な伝え方」の教科書』の感想文を書いていきます。
心構え編の第1弾は↓こちら↓
第2弾は、「伝わるコードレビューの5大ルール」について書いていきます。
コードレビューにおいてレビュイーとレビュアーに必要なものは？
コードレビューはソフトウェア開発において、コードの品質を保つために必要なプロセスです。しかし、コードレビュー内でのコミュニケーションに齟齬が生じてしまっては効果が薄れていきます。
また、基本的に開発には締め切りがあるため、効率よく開発を進める必要があります。そのため、コミュニケーションがうまくいかないと、そこに時間を取られ、プロジェクト全体の進みが遅れ、締め切りに間に合わないという現象が発生しかねません。
だからこそ、開発を円滑に進めるためには、「コードレビューにおける指摘や質疑応答がスムーズに行われること」が重要です。ここで、レビュイーとレビュアーに必要なのは、「伝達ロスが少なく、意図が正確に伝わるコミュニケーション」が必要です。
それを実現するためのルールがこちらです！
伝わるコードレビューの5大ルール
- 決めつけない
- 客観的な根拠に基づく
- お互いの前提知識を揃える
- チームで仕組みを作る
- 率直さを心がける
1. 決めつけない
まず１つ目は、「決めつけない姿勢を持つこと」です。
開発者がどれだけ熟練者だとしても、使用したことのない言語を使っていれば「基本的な文法」や「推奨されない書き方」などはわかるはずありません。開発者が初心者なら尚更です。そのため、レビュアーは開発者が「言語について・文法についてわかっているだろう」と決めつけてレビューをするのは良くないことです。
ハンロンの剃刀
また、本書では、「ハンロンの剃刀」というメンタルモデルを挙げていました。これは以下の意味を持つメンタルモデルです。
Never attribute to malice that which is adequately explained by stupidity.
（無能で十分説明されることに悪意を見出すな）
むむむ...
つまりはこういうことでしょうか？
あるバグが間違いや見落としで説明できる時、悪意があると決めつけないようにしろ
ちょっとわかりづらかったので、例を考えてみました。
以下のようなRubyコードがあるとします。（Rubyistなので...恐縮です...）
hash = { a: 1, b: 2, c: 3 }
hash.values.each do |v|
p v
end
# 出力結果
# 1
# 2
# 3
このコードに対して、悪意のあるレビューをするとしたら、「なんでeach_valueメソッドを使わないんですか？」ですかね？
このレビューを悪意0に変換すると、「valuesメソッドとeachメソッドを繋げるのも良いですが、each_valueメソッドであれば1つのメソッドで完結しますが、いかがでしょうか？」ですかね？
推測ではなく確認をしましょう！
また、コミュニケーションに齟齬が生じてしまう原因として、「このコードはこういう意図で書いているんだろうな」と推測してしまうことも良くないです。もし、それが違ったらコードに対する認識が異なってしまいます。
開発を円滑に行うためにも、推測はせずに開発者にコードの意図を確認するようにしましょう。
2. 客観的な根拠に基づく
コードレビューにおいて、自分の意見を伝える場合には必ず「客観的な根拠を伝えること」が大切です。
これは推測をしないに近しいのかもしれないですが、「多分」「もしかしたら」「かもしれない」というような断定できていないレビューには、根拠がありません。勘です。
ここでいう根拠というのは、実際に手元でコードを動かした時に「この引数を渡された時にエラーが発生しているようです」「このメソッドが実行された時にnullが返ってきているようです」などの証拠のことだと自分は思います。
レビュアーは手間ですが、動作確認をした時に想定外の動作をしていたら「ここのコードで○○の動作をしてしまっているので、このようにしてみてはいかがでしょうか？」というように、実際のコードの動作を踏まえた上でレビューをするのが良いのだと考えました。
正解が存在しないケースもある
プログラミングの世界では、正解が存在しないケースもあります。上記のコードでも、.values.eachのようにメソッドを連結して使うことが間違いだとは言い切れません。each_valueメソッドよりも.values.eachの方が好みである開発者もいらっしゃいます。
この場合は、「根拠はないのですが、私は〇〇よりも△△の方が好みです」と伝えることで、「絶対に直す必要はない」という意図を伝えることができます。
3. お互いの前提知識を揃える
例えば、Aさんは開発環境をDockerで構築しているが、BさんはDockerを使用せずに開発環境を構築しているとします。この環境の違いによって、パフォーマンス面の違いだったりDockerの容量不足のによるエラーの有無などがあったりします。
そのため、Bさんの環境で「この操作をした時にこんなバグが出ました！」というものがAさんの環境では発現しなかったり。その場合には、原因を一緒に突き止めることが難しかったりするので、開発サイクルに悪影響を及ぼすこともあります。
上記のように特定の条件下で発生するバグもあるため、開発チームのメンバーが「どのような条件かで開発をしているのか」「どのような知識を有しているのか」などを把握しておく必要があります。
また、開発メンバーの今までの環境によって、用語や概念が違うことも多々あります。これもそのままにしておかずに、認識を揃えておく必要があります。
4. チームで仕組みを作る
前述した内容やこれから伝える内容は「個人の努力」だけでは全ての問題を解決するのは難しいです。また、私たちは人間なので、「詳細に書こう！」という日もあれば「このくらいでいいや」って思う日もありますよね。そうすると、プルリクエストやレビューの内容にムラが生じてしまいます。
ムラが生じないためにも、「意識せずとも事前に正しい行動を取れる仕組みを作ること」は大切です。仕組みの例として以下が挙げられていました。
- プルリクエストのディスクリプション（詳細）のテンプレートを用意する
- レビューのガイドラインを用意する
- レビューのチェックリストを用意する
「心構え編」の第1弾でも記述しましたが、以下の内容を自然に記入するためのテンプレートがあると良いです。
- このコードによって実現されるべき状態・仕様
- 変更の前後での違い
- なぜこのような実装にしたのかの設計判断
- レビューで検討してほしい点
- 動作確認方法
上記をテンプレート化しておくことによって、プルリクエストのディスクリプションにムラが発生しづらくなりますね。
また、レビューを行う際のチェックリストを設けておくのも良いですね。マークダウン記法でチェックリストを作成することも可能ですし、テンプレートの中にチェックリストを作成しておけばより良いですね。
* [ ] 項目A
* [ ] 項目B
5. 率直さを心がける
チーム内での信頼関係を築き、プロジェクトを成功に導くための重要な要素として、「物事を率直に伝えること」は必要です。ここでいう「率直さ」というのは、「相手に対する敬意を忘れず、伝えるべきポイントを的確に、かつ簡潔に伝えること」を意味しています。
なんでもかんでもストレートに伝えることで、相手が傷つくこともあります。ただし、過度に言葉を柔らかくしすぎて意図が伝わらないということもあります。
逆に、レビュイー側も率直さを持つ必要があります。なんでもかんでも受け入れるのは良くないですが、もらったレビューに対して「私は間違っていない」という姿勢を持ち続けるのも円滑なコミュニケーションにはつながらないですよね。レビューをもらった時は、素直に「こういう書き方があるのか！ありがとうとうございます！」という気持ちを持つことが必要ですよね。その上で自分の考えを伝えることが、チーム内での信頼関係を築き、プロジェクトを成功に導くための重要な要素なんだと思います。
まとめ
以下のルールに則ってコードレビューを行えるようになろう！
- 決めつけない
- 客観的な根拠に基づく
- お互いの前提知識を揃える
- チームで仕組みを作る
- 率直さを心がける
Discussion