公開日
コードレビューの生産性向上：リファクタリングを誘発するコメント術
コードレビューは、ソフトウェア開発において、バグの削減やコード品質の向上など、様々なメリットをもたらします。近年では、コードレビューを通じて、リファクタリングを促し、コードの保守性や可読性を高めることへの期待も高まっています。しかし、具体的にどのようなレビューコメントがリファクタリングを効果的に誘発するのか、そのノウハウは十分に共有されているとは言えません。
本記事では、2024年10月に発表された論文「A qualitative study on refactorings induced by code review」(Coelho et al., 2024)に基づき、コードレビューの生産性向上という観点から、リファクタリングを誘発するレビューコメントのガイドラインについて解説します。
コードレビューがリファクタリングを促進するメカニズム
リファクタリングはなぜコードレビューで生まれるのか？
コードレビューは、開発者が作成したコードを、他の開発者がチェックするプロセスです。このプロセスの中で、レビュアーはコードの問題点を指摘し、改善提案を行います。このときの、改善提案が、リファクタリングの実施のきっかけとなるケースが多く報告されています。 実際、本研究によると、分析対象となったPRのうち、実に50%ものPRでコードレビューがリファクタリングを誘発したことが明らかになっています。
| リファクタリング誘発要因 | 割合 |
|---|---|
| コードレビューのみ | 50% |
| 作者のみ | 26.7% |
| コードレビューと作者 | 23.3% |
つまり、コードレビューは、単にバグを発見するだけでなく、コードの品質を高めるためのリファクタリングを促す、強力なメカニズムであると言えるのです。
データで見る！リファクタリングを伴うPRの特徴
このセクションでは、リファクタリングを伴うPRと伴わないPRを比較し、どのような違いがあるのか、データを見ながら明らかにします。リファクタリングを伴うPRの特徴を理解することで、どのようなコードレビューが効果的なのかが見えてきます。
本研究では、リファクタリングを伴うPRと伴わないPRを比較し、興味深い違いを発見しました。
以下の表、リファクタリングを伴うPRは、伴わないPRと比べて、ファイル変更数、追加行数、削除行数が多いことが分かります。
目的サンプルの概要
| リファクタリングを誘発するPR数 | リファクタリングを誘発しないPR数 | レビューコメント数 | 後続コミット数 | リファクタリング編集数 |
|---|---|---|---|---|
| 10 | 10 | 100 | 40 | 66 |
以下の表にて、リファクタリングを伴うPRと伴わないPRをそれぞれ無作為に10個ずつ抽出して、コードレビューに関する統計情報を比較しました。リファクタリングを伴うPRは、伴わないPRと比べて、ファイル変更数、追加行数、削除行数すべてにおいて、平均値・中央値ともに、大きな値を示しています。 特に、追加行数と削除行数においては、その差が顕著です。
目的サンプルにおける統計概要 平均値(と中央値)
| リファクタリングを誘発するPR | リファクタリングを誘発しないPR | |
|---|---|---|
| レビュワー数 | 2.0(2.0) | 2.0(2.0) |
| コメント数 | 5.0(5.0) | 5.0(5.0) |
| マージまでの日数 | 3.4(1.0) | 2.5(1.0) |
| 後続コミット数 | 2.3(2.0) | 1.7(1.5) |
| ファイル変更数 | 9.1(5.5) | 3.0(2.0) |
| 追加行数 | 103.9(31.0) | 21.8(8.5) |
| 削除行数 | 50.5(23.0) | 14.8(7.5) |
これは、リファクタリングが、コードの構造や設計に関わる、比較的大規模な変更を伴うことが多いことを示唆しています。つまり、リファクタリングを多く含むPRは、変更規模が大きいため、より丁寧なレビューと、発な議論が必要となることが推測されます。
経験豊富なレビュアーが果たす役割とは
本研究で、特に注目すべき点は、経験豊富なレビュアーの貢献です。データ分析の結果、リファクタリングを伴うPRでは、経験豊富なレビュアーが、経験の浅いPR作成者に対して、より丁寧かつ具体的なコメントで、リファクタリングを促していることが明らかになりました。
例えば、経験豊富なレビュアーは、単に「この部分はリファクタリングが必要です」と指摘するだけでなく、「なぜリファクタリングが必要なのか」という理由を丁寧に説明したり、具体的な改善方法を例示したりすることで、PR作成者の理解を深め、自発的なリファクタリングを促しています。
これは、経験豊富なレビュアーが、コードの品質を高めるための、豊富な知識と、それを効果的に伝える技術を有していることを示しています。つまり、経験豊富なレビュアーの知見を、コードレビューを通じて、チーム全体に共有することが、コードレビューの生産性を高める上で、非常に重要であると言えるでしょう。
リファクタリングを誘発するレビューコメントの特徴
では、具体的にどのようなレビューコメントが、リファクタリングを誘発しやすいのでしょうか？本研究では、リファクタリングを誘発したレビューコメントと、誘発しなかったレビューコメントを比較し、その特徴を分析しています。
具体的かつ丁寧な指摘が開発者を動かす
リファクタリングを誘発するレビューコメントに共通する特徴として、まず挙げられるのは、具体的かつ丁寧な指摘です。
効果的なコメント例
例えば、以下の画像は、 apache/tinkerpop#1110 のPRにおけるレビューコメントの例です。ここでは、RequestOptions を Bytecode から抽出することを、非常に具体的かつ丁寧な表現で提案しています。 このようなコメントは、PR作成者にとって、何を、どのように変更すれば良いのかが明確であり、リファクタリングの実施を後押しする効果があります。
以下は、リファクタリングを誘発したレビューコメントの特徴をまとめたものです。このリストからも、コードロジックに関する質問や、具体的な改善提案が、リファクタリングの誘発に繋がっていることが分かります。
- コードロジックに関する質問
TIMEOUT_FILTER_START_TIMEという添付ファイルがRPCチェーン全体に渡されてしまうので注意してください。これは RpcContext の欠点です。 ( apache/dubbo#3174 )- 任意のバイト列に対して toString をここで呼び出しても安全ですか？ ( apache/fluo#837 )
- コード改善の提案
keystorePathにStringの代わりにPathを渡せませんか？ ( apache/knox#69 )- 英語で新しいコメントを書くことをおすすめします ( apache/servicecomb-java-chassis#346 )
ConfigProviderで終わらない別のケースも追加してみてください。 ( apache/kafka#5194 )
- 良い開発プラクティスに関する提案
- 良くなりましたが、左揃えの中央にできますか？今は左下になっているようです。 ( apache/cloudstack#3454 )
private static finalは、通常すべて大文字です。 ( apache/brooklyn-server#1049 )- 特に複数の列挙型を比較する場合は、 if else の代わりに switch case を使用した方が良いでしょう ( apache/cloudstack#2833 )
避けたいコメント例
一方、下記は、リファクタリングを誘発しなかったレビューコメントの特徴をまとめたものです。このリストを見ると、コードの見た目に関する指摘や、抽象的で意図が伝わりづらいコメントなど、表面的で重要度の低い指摘が多いことが分かります。
- コードの見た目に関する指摘
- このセクションのコードフォーマットが異なります（スペースが使用されています） ( apache/cloudstack#3430 )
- 曖昧なコメント
- IllegalStateException の方が良い？ ( apache/servicecomb-java-chassis#691 )
- 116行と118行にも元に戻すべき変更があります。 ( apache/kafka#5111 )
- 例外メッセージの一部にアサーションを追加してみてはどうでしょう？ ( apache/beam#6317 )
- 修正の強要
- このメソッドは以下のように修正すべきです
protected int ...( apache/dubbo#4870 )
- このメソッドは以下のように修正すべきです
これらのコメントは、コードの品質を本質的に向上させるものではなく、リファクタリングの動機付けとしては弱いと言えます。また、不明瞭な表現を用いたり、修正を強要したりするようなコメントは、PR作成者のモチベーションを下げ、リファクタリングを遠ざける可能性があります。
成功の鍵は「根拠」と「例」にあり
リファクタリングを誘発するレビューコメントの、もう一つの重要な特徴は、 「根拠」と「例」 です。以下の画像は、 apache/brooklyn-server#1049 のPRにおけるレビューコメントの例です。ここでは、定数をすべて大文字で記述するというコーディング規約を根拠として、maxRetryCount のリファクタリングを提案しています。
このように、なぜリファクタリングが必要なのか、その根拠を明確に示すことで、PR作成者は、変更の必要性を納得し、リファクタリングを前向きに捉えることができるようになります。
また、具体的な「例」を示すことも非常に効果的です。例えば、以下の画像は、 apache/kafka#5946 のPRにおけるレビューコメントの例です。ここでは、具体的なコード例を挙げて説明することで、PR作成者は、レビュアーの意図を正確に理解し、よりスムーズにリファクタリングを実行することができます。
「直接的な提案」がリファクタリングを後押しする
本研究では、直接的なリファクタリングの提案が、PR作成者に受け入れられやすいことも明らかになっています。以下の画像は apache/dubbo#3299 のPRにおけるレビューコメントの例です。ここでは、“is an array list here a little bit over-kill?” と、ArrayList の使用が過剰ではないかと、型の変更を提案しています。
このように、「～してはどうですか？」と、具体的な変更方法を直接提案することで、PR作成者は、リファクタリングの具体的なイメージを掴みやすくなり、変更への心理的ハードルが下がる効果が期待できます。
レビューコメント作成のための実践的ガイドライン
以上の分析結果を踏まえ、本研究では、リファクタリングを誘発するレビューコメント作成のための、実践的なガイドラインを提案しています。
研究から導き出された5つの指針
- 礼儀正しく、前向きな表現を心がける: “can we …?”, “maybe …” のような、丁寧な表現でリファクタリングを提案しましょう。
- 具体的かつ明確に指摘する: 抽象的な表現は避け、問題の箇所と、期待する改善後の状態を具体的に記述しましょう。
- 根拠や例を示す: なぜ変更が必要なのか、その理由を明確に示すことで、PR作成者の納得感を高めましょう。具体的なコード例を挙げることも効果的です。
- 質問を活用する: 一方的な指摘ではなく、質問を投げかけることで、PR作成者に問題点を自ら考えさせることも有効です。
- 強制は避ける: 修正を強制するのではなく、あくまで提案としてコメントすることが重要です。特に、埋め込みコードをする場合は、強制するのではなく提案であることを伝えましょう。
あなたの現場でのガイドライン活用法
これらのガイドラインは、あくまで一般的な指針です。実際の開発現場では、プロジェクトの性質やチームの文化などに合わせて、適宜修正を加えて活用することが重要です。
例えば、チームメンバー全員が、リファクタリングの重要性を理解し、積極的に実施することに合意しているような、成熟したチームであれば、より簡潔なコメントでも、リファクタリングが促されるかもしれません。一方、経験の浅いメンバーが多いチームでは、より丁寧な説明や、具体的な例示が求められるでしょう。
また、ガイドラインを、チームのルールとして明文化することも効果的です。ルールを明確にすることで、レビューコメントの質のばらつきを抑え、チーム全体で一貫したレビューを行うことができるようになります。 さらに、定期的にガイドラインを見直し、現場の実態に即した、より実効性の高いものに更新していくことも重要です。
結論
本記事では、コードレビューの生産性向上という観点から、リファクタリングを誘発するレビューコメントのガイドラインについて解説しました。コードレビューは、リファクタリングを促進する効果的な手段であり、特に、経験豊富なレビュアーによる、具体的かつ丁寧なコメントが、リファクタリングの誘発に大きく貢献することが分かりました。また、「なぜ変更が必要なのか」という根拠を明確に示すことや、具体的な例を挙げて説明することも、効果的なレビューコメントに共通する特徴です。
これらの知見を、本記事で紹介した 「リファクタリングを誘発するレビューコメントのガイドライン」 として活用することで、より 生産性の高いコードレビュー が実現できるでしょう。その結果、ソフトウェアの品質向上、開発効率の改善、そして、開発者の成長といった、様々な効果が期待できます。
開発生産性やチームビルディングにお困りですか？ 弊社のサービス は、開発チームが抱える課題を解決し、生産性と幸福度を向上させるための様々なソリューションを提供しています。ぜひお気軽にご相談ください！
参考資料: