Edited at

【和訳】コードレビューのベストプラクティス

株式会社オズビジョンのユッコ (@terra_yucco) です。

本記事は 2017 年に書いたものですが、2019 年に姉妹記事を翻訳したことで、記事のタイトルやタグ、その他本文中のインデントなどを一部修正しています。

内容については変更はありませんが、ご了承ください。

上記 SmartBear Collaborator 説明前提記事のうち、一番キャッチーな Best Practices for Code Review が本記事の翻訳元です。


[以下、初版で書いていた冒頭文]

現在、コード改善活動をエンジニア一丸となって進めています。その取り組みの中には、コード規約を作ったり、ペア作業をしたり、レガシーコードにテストコードを網羅していこうだったり、色々なものがあります。その中の一つに、レビューの有効活用があげられています。

今回、チームメンバーが良い記事をシェアしてくれましたので(英語だけど)、読みながら私的に翻訳したものを共有したいと思います。ちなみに単語などは正確性よりも、全体の印象を強化するものを選びました。



Source Article

以下に記載の文章は、最後の Conclusion 部分を除き原文の内容をユッコの英語力の範囲で訳したものです。ただし、正確に一言一句訳すわけではなく、かなりの意訳を挟んでいます。

また、貼り付けられている図表類は全て上記記事からお借りしています。


前書き

コードレビューをうまく行うためのピアレビュー戦略には、厳密なプロセスと、気軽に行える環境のバランスが大事。ピアレビューをガチガチにやりすぎると生産性は低下するけれども、中途半端にやってもあまり効果がない。

マネージャには、チーム内のオープンなコミュニケーションや知の共有を促進しつつ、ピアレビューが効率的に効果をあげる地点を見つける責任がある。


効果的なピアコードレビューを行うための 10 の Tips


  • 表の名称: 欠陥密度 vs コード行数

  • 縦軸:欠陥密度 (欠陥数/ k 行数 (コード 1,000 行が 1))

  • 横軸:レビュー対象コード行数

※訳注:見るコードの量が少ないほど、欠陥の検出度合いが良い。ということっぽい。


1. 1 回でレビューするコードは400 行より少なくしよう

シスコシステムズのプログラミングチームに対する SmartBear の調査では、開発者が一度にレビューするべきコードの量は 200 - 400 行。脳は大量の情報を一度に処理することができるけれど、400 行を超えると欠陥が見つかりにくくなる。

実際、200 - 400 行のコードを 60 - 90 分かけてレビューすれば、70 - 90% の欠陥を発見できる。コードの中に 10 個の欠陥がある場合、適切なレビューを行えばうち 7 - 9 個は見つかる。


  • 表の名称:欠陥密度 vs 検査率

  • 縦軸:欠陥密度 (欠陥数/ k 行数(コード 1,000 行で 1))

  • 横軸:レビューの検査率 (コード行数/時間)

※【訳注】時間あたりのコード行数が増えると、検出できる欠陥数は減るということか? Defect Density は検出できる欠陥密度を指しているように読める。

→後ろの方読んだらこれで合っていた。


2. レビューには十分時間をかけよう。検査率は 500 行/時間が妥当

あなたが見つけられない欠陥は他の誰かが見つけるだろうという仮定で、レビューはさっさとやりたいと思うかも。

しかし、SmartBear による結果では、500 行/時間以上の速度でのレビューでは欠陥[検出]密度が大幅に下がることがわかっている。コードを「意味のある単位で」「ゆっくり」「適切なペースで」読むことで、最も効果的なレビューができるということ。


3. 1 回に 60 分以上レビューしちゃダメ

コードをあまりに速くレビューしてはいけないのと同様、1 回に長時間のレビューもするべきではない。

集中を要する仕事を行っていると、約 60 分後にはパフォーマンスが下がってくることが知られている。一定期間ごとに休憩を入れることで、仕事の質が大幅に向上することもわかっている。


4. ゴールを設定し、メトリクスを計測しよう

プロセスに沿ってレビューを行う前に、以下をやるべき。


  • ピアレビューの有効性をどのように計測するか決める

  • 目に見えるゴールを設定する

設定には SMART 基準を使用し、外部メトリクス(外から見てわかる値)から設定していく。例えば「サポート問い合わせを 15% 削減する」または「開発作業で作り込まれた欠陥を半分にする」など。これにより、(レビューにより)コードがどのように改善されたかが定量化できる。「より多くのバグを修正する」は効果的なゴールではない。

※訳注;SMART 基準とは >> 目標設定は「SMART」に


◆要素1:Specific(具体的に)

誰が読んでもわかる、明確で具体的な表現や言葉で書き表す

◆要素2:Measurable(測定可能な)

目標の達成度合いが本人にも上司にも判断できるよう、その内容を定量化して表す

◆要素3:Achievable(達成可能な)

希望や願望ではなく、その目標が達成可能な現実的内容かどうかを確認する

◆要素4:Related(経営目標に関連した)

設定した目標が職務記述書に基づくものであるかどうか。と同時に自分が属する部署の目標、さらには会社の目標に関連する内容になっているかどうかを確認する

◆要素5:Time-bound(時間制約がある)

いつまでに目標を達成するか、その期限を設定する


また、以下を含む内部プロセスのメトリクスの計測も効果的。


  • 検査率:レビューを行う速度

  • 欠陥率:レビュー 1 時間あたりで発見されたバグの数

  • 欠陥密度:コード 1 行あたりで発見されたバグの平均数

実際のところ、冪等性を持ったメトリクスを出せるのは、自動、または厳密に管理されたプロセスのみ。メトリクス指向のコードレビューツールでは、データを自動的に収集するため、正確な情報を得ることができ、人によるバイアスがかからない。

※【訳注】ツールの宣伝記事でもあるっぽいので、このあたりは読み飛ばす。


5. 作成者はレビュー前にコードに注釈をつけよう

作成者は、レビューの前にコードに注釈をつけるべき。なぜなら、注釈によりレビューアに「変更」「最初に見るべきもの」「コード変更の理由」などを示すことができるから。

注釈は他のレビューアの作業を楽にし、より深いコンテキストを提供する役に立つ。更なるメリットとして、ピアレビューが始まる前に作成者自身が欠陥に気づけることもある。

ピアレビュー前により多くのバグを見つけておけば、残存バグ数が減り、結果的に欠陥密度が下がる。


6. チェックリストを使おう

チームメンバーが同じ 10 個の間違いを繰り返す可能性は、とても高い。特に「欠落」は、「そこに無いものを見つける」ということの難しさから、検出が最も難しい欠陥と言われている。

繰り返される間違いを排除し、「欠落は見つけにくい」という課題に対処するためには、チェックリストの利用が最も効果的。


7. 欠陥を修正する方法を確立させよう

タイムボックスをきちんと守ったレビュープロセスが適切に行われるようになった。時間あたりに適切な量のコードをレビューするようになった。チームにとっての重要メトリクスが定義された。としても、まだ足りないステップがある。

バグが見つかった時に、どのように直すの? そんなの明白じゃないと思うかもしれないけど、多くのチームは、見つかりにくいバグを修正するためのシステマチックな方法を確立していない。

欠陥を確実に修正するためのベストプラクティスは、レビューアがバグを記録し、作成者と議論をし、コードの変更を承認できる「collaborative code review tool」を使うこと。こういったツールを使っていない場合、レビューで見つかったバグは品質管理(QA)に回る前に出たものであるため、チームのバグトラッキングシステムに記録されない可能性がある。


8. ポジティブなコードレビュー文化を育てよう

ピアレビューはチームの対人関係に影響することがある。コードの全ての批評をチームメイトが行い、管理者がコードの欠陥密度を測定・評価するのは難しい。そのため、ピアコードレビューがうまくいくには、ピアレビューによる協働と学習の文化を作ることがとても大事。

欠陥=「悪いもの」として見ることは簡単。ただし、バグはチームがコードの品質を改善する機会でもある。また、ピアレビューは、経験の浅いチームメイトがリーダーから学ぶ良い機会になるし、経験豊富なプログラマの悪癖を直すきっかけにもなる。

ピアレビューで見つかった欠陥をチームメンバーの評価に使うたぐいのものではない。ピアコードレビューの結果を、パフォーマンスの評価として使わないこと。個人の指標が報酬や昇進の基準になると、開発者はプロセスに敵意を抱くようになる。そして、全体が良くなるコードではなく、個人的なメトリクスを改善するためのコードを書くようになってしまう。


9. ピアレビューを意識させる

自分の仕事を誰かがチェックすると知っていると、人は自然により良い仕事をしようとする。実際にチームメイトが成果を見るので、エゴ効果により開発者は自然とよりきれいなコードを書くようになる。

シスコシステムズの SmartBear の調査では、コードの 20 - 33% をスポット的にチェックすることで、掛ける時間は最低限で欠陥密度を下げられることがわかっている。あなたのコードの 1/3 がレビューされていれば、仕事のダブルチェックとしてはもう十分。


10. 軽いコードのレビューで練習しよう

メールでも肩越しでも、MS Word でも。さまざなツールを利用したハイブリッド開発では、コードを共同でにレビューする方法は無数にある。ただし、チームの時間を効果的に使い、その結果を有効に測定するには、ツールを利用した軽量なプロセスがおすすめ。

シスコシステムズの SmartBear の調査によると、軽量コードレビューでは正式なレビューの 20% の時間しかかけないにもかかわらず、多くのバグを検出できた。きちんとした検査は、200 行あたり 9 時間かかり、これはこれで多くの場合効果的。だが、ガチガチにやろうとすると、最大 6 人の参加者が数時間の会議を行い、詳細なコードのプリントアウトのページをめくることになる。


Conclusion

[2017-04]

技術英語の翻訳って難しいんですね!

洋書を翻訳してくださる技術者の方を心から尊敬したくなりました。

読み始めたら面白かったので一気に翻訳はしましたが、こういうドキュメントをもっと気軽に読めるようになりたいものです。

[2019-05]

改めて自分の拙い訳とともに読み返すと、やはり新しい学びがありました。

あまり多い量のコードを読んでいるわけではありませんが、やはり続けてレビューするのは 1 時間が限界ですし、気になる点に絞ってのライトなレビューは効果が高いように感じます。

チェックリストも明確には作っていませんが、私も含めたチームメイトは、みな自分なりのレビュー基準を持っているように感じます。

周辺ドキュメントの翻訳は、今後も折に触れて続けていきたいと思います。

※ただし、SmartBear は使っていませんし、コードレビューは今のところ GitHub の Pull Request で十分だと感じています :smiley: