コードの可読性についてのプレゼンテーション紹介 vol. 5: “レビューとまとめ” 編

はじめに

こんにちは。コミュニケーションアプリ “LINE” の Android クライアントチームの石川です。

この記事は、 “コードの可読性についてのプレゼンテーション紹介” の不定期連載記事の第五回です。
前回の記事はこちら です。

今回はいよいよ最終回です。第八章の “レビュー” についての解説と、本連載のまとめをします。

第八章: レビュー

持続可能なソフトウェア開発を行うための一手段として、コードレビューで可読性や頑健性を向上させることは効果的です。コードレビューを行うことで、その時点でのコードの品質向上に寄与するだけでなく、情報共有によるチーム全体の技術力の向上も見込まれます。ただし、コードレビュー自体が開発のボトルネックにならないように気をつけるべきです。この章では、コードレビューにおける注意点を、レビューを依頼する側とレビューをする側の両方の観点から解説します。

なお、本記事ではレビューツールとして GitHub を使うことを想定しています。GitLab や Gerrit 等、他のコードレビューツールを使っている場合は、 “プルリクエスト” をそれぞれ “マージリクエスト” や “チェンジ” 等、適宜読み替えてください。

1.  コードレビューを依頼する

1-A: プルリクエストとコミットは小さく構造的に

プルリクエストやコミットの意図が明確であれば、コードレビューが効率的になります。そのためには、プルリクエストやコミットを構造的にしなくてはなりません。例えば、名前の変更のように単純かつ広範囲な変更と、ロジックの変更のように局所的かつ複雑な変更が一つのコミットに入っていると、レビューをするべき箇所の理解に時間がかかります。レビューを依頼する前にコミットの分割や並び替えを行い、各コミットの責任範囲とコミット間の流れを明示的にしましょう。

また、大きなプルリクエストやコミットも、開発速度の低下の要因になりえます。大きなプルリクエストはレビューに時間がかかるだけでなく、修正が必要になった場合に大きな手戻りが起きかねません。一週間で大きな一つのプルリクエストを作るのではなく、一日のタスクを複数のプルリクエストに分割すべきでしょう。プルリクエストを分割するためには、一時的に完動しないコードをマージする必要もあります。例えば、先に部品となるモデルやユーティリティのみを先に作っておいたり、依存関係を示すためだけのスケルトン実装を作ると良いでしょう。その際は、今後の実装の予定を、TODOコメントやプルリクエストコメントで明示すると良いでしょう。

1-B: レビューコメントの意図と理由を理解する

レビューコメントをプルリクエストに反映させる際には、そのコメントの意図と理由を理解するということが重要です。例えば、レビュアーが勘違いしてコメントをしたり、コメント内で質問をしている場合は、コードの難解さがその誤解や疑問を招いている可能性を考慮しましょう。その場合は、コードの変更を用いてレビューコメントに返信することができます。例えば、より適切な名前に変更したり、コメントを追加するだけでも、誤解や疑問を解消できることがあります。

2.  コードレビューを行う

2-A: レビューを拒否することの重要性

コードレビューを円滑に行うため、レビュー依頼から最初の返信をするまでの目標時間を、プロジェクトで定義しておくと良いでしょう。目標時間の設定例として、”レビュー依頼を受けた時点から翌勤務日の同時刻までに返信する” といったものが挙げられます。この目標時間は、あくまでも返信までの時間であり、レビューを行うまでの時間ではないことに注目してください。例えば、他に優先すべき作業によってレビューができない場合、”現在レビューできない” もしくは “レビューが遅れる” と返信をするべきです。そうすることで、レビュー依頼者が次の行動を選択することができます。例えば、プルリクエストの緊急性が高い場合は他の開発者にレビューを依頼することができますし、緊急性の低い場合はそのままレビューを待つという選択が取れます。

また、プルリクエストが巨大であったり、設計に根本的な欠陥がある場合、そのままレビューを続けると莫大な時間を消費してしまう可能性があります。その場合は、詳細なレビューをせず、一旦プルリクエストをクローズしてもらうのも一つの手段です。その後、どうプルリクエストを分割すればよいかやどう設計すればよいかを、チャットやペアプログラミング等を用いてフォローアップすると良いでしょう。このようにすることで、結果的に生産性を向上できる場合が多いです。

2-B: 何をコメントするべきか

レビュアーがなにを見るべきかについて、以下のような点が挙げられます。

  • スタイル, コーディング規約, 言語やプラットフォームにおけるイディオム (CI でチェック可能)
  • テストの範囲と妥当性
  • このプレゼンテーションの内容すべて: 原則, 命名, コメント, 状態, 手続き, 依存関係
  • その他見つけた問題なんでも

なお、問題点を指摘する場合、修正方法までコメントするかについては一考すべきです。例えば、修正の内容は依頼者に考えてもらうことで、依頼者とレビュアーの負担のバランスをとったり、相互の技術力を向上できる場合があります。状況により、修正内容の提示・問題の指摘・質問を使い分けると良いでしょう。

本連載のまとめ

これまで、本プレゼンテーションにおける全八章の概要を解説してきました。最後に、各章の要点をまとめます。

第一章: 導入と原則

  • 可読性は継続的な開発のため
  • The boy scout rule, YAGNI, KISS, single responsibility principle, premature optimization is the root of all evils

第二章: 命名

  • 対象が何であるか・何をするのかを名前で示す
  • 文法・語の選択に気をつける

第三章: コメント

  • コメントはコードの理解の補助と誤解の防止のため
  • ドキュメンテーションでは要約を書く

第四章: 状態

  • Non-orthogonal な関係を排除する
  • より単純な状態遷移 (不変・一方向) を使う

第五章: 手続き

  • 手続きの責任範囲 (副作用・返り値) を明示的にする
  • Definition based programming で流れを示す

第六章・第七章: 依存関係

  • 内容結合・共通結合・制御結合を緩和する
  • 依存関係の循環・重複を避ける
  • 明示的な依存関係を使う

第八章: レビュー

  • プルリクエスト・コミットは小さく構造的にする
  • レビューコメントの意図を意識する
  • レビュアーとして無理をしない

このプレゼンテーション では、今回の連載では書ききれなかった話題や具体例についても触れているので、ぜひご参照ください。

これで、コードの可読性についてのプレゼンテーション紹介の不定期連載は完結です。ありがとうございました。

Related Post