Posted inDevelopment

効果的なコードレビュー：新規開発者のコード品質とプログラミングスキルを向上させる

Development tutorial - IT technology blog
Development tutorial - IT technology blog

クイックスタート：5分でコードレビュー

同僚からプルリクエスト（PR）を受け取り、メインブランチにマージする前にコードを確認してほしいと頼まれました。心配しないでください！すぐに始める方法を説明します。

  1. プラットフォーム（GitHub, GitLab, Bitbucket）でPRを開く： 通常、リンクが送られてくるので、それをクリックします。

  2. PRの説明を読む： 同僚が何をどのように変更したいのかを確認します。これは変更の背景を理解するのに役立つ要約です。

  3. 変更点を確認する（Files changed）： 各ファイルを一つずつ確認します。新しく追加されたコード行、修正または削除された部分に焦点を当てます。GitHubのようなツールでは、これらの変更が緑色（追加）または赤色（削除）でマークされることが多いです。

  4. コメントを残す： 不明な点、改善できる点、またはバグを見つけた場合は、そのコード行に直接コメントを残してください。見つけた問題を具体的に記述するように努めます。

    # GitHub/GitLabでのコメント例
# @username: このif条件は少し複雑に見えます。可読性を高めるために、簡素化するか、別の関数に分割できますか？
# または: statusがnullの場合の処理ロジックがまだ処理されていないようです。ここでnullチェックを追加できますか？

  5. フィードバックを送信する（Submit review）： 確認後、「Approve」（承認）、「Request changes」（変更要求）、または「Comment」（一般的なコメントのみ）を選択できます。深刻な問題がある場合は、「Request changes」を選択してください。

これで基本的なコードレビューは完了です！では、さらに深く掘り下げていきましょう。

コードレビューとは何か、そしてなぜそれが重要なのか？

コードレビュー、または「ソースコードレビュー」とは、一人または複数の他の開発者があなたのソースコードをレビューし、エラーを見つけ、品質を向上させ、プロジェクトの標準を満たしていることを確認するプロセスです。これは「粗探し」ではなく、お互いに学び合い、より良い製品を共に作り出すための協力的な活動です。

なぜコードレビューはこれほどまでに重要なのか？

  • ソースコードの品質向上： 二つの目は一つよりも優れています。レビュー担当者は、論理エラー、潜在的なバグ、または見落とされたシナリオを発見することができます。これにより、コードが本番環境にデプロイされた際のエラーを最小限に抑え、不必要な問題を防ぐことができます。

  • 知識と経験の共有： 他の人のコードをレビューすることで、彼らが問題をどのように解決し、どのようなデザインパターンを使用しているかを学ぶことができます。逆に、他の人があなたのコードをレビューすることで、ソフトウェアについてさらに理解が深まり、その結果、チームは特定の個人に依存することなく、より強くなります。

  • 標準への準拠の確保： 各プロジェクト、各チームには、コーディングルール（コーディングスタイル、命名規則）があります。コードレビューは、全員がこれらルールの遵守を確実にし、ソースコードの一貫性を保ち、読みやすく、保守しやすい状態を維持するのに役立ちます。

  • 早期のバグ発見と修正： レビュー段階でバグを修正することは、製品がデプロイされてから発見するよりもはるかに簡単で安価です。デプロイ後のバグはユーザーに影響を与え、修正に多くの時間と労力を要します。

  • プログラミングスキルの向上： これは自己成長のための最速の方法の一つです。自分のコードが他の人に読まれることを知っていると、自然と慎重に、明確に、そして構造的にコードを書くようになります。他の人のコードをレビューする際には、批判的に思考し、彼らの解決策から学び、それによって知識と経験を広げることができます。

最近の5人の開発者が関わったウェブアプリプロジェクトで、私とチームはより厳格なコードレビュープロセスを適用しました。その結果、デプロイ後に修正する必要があるバグの数が大幅に減少し、古いバグを修正するために戻る必要がほとんどなかったため、新機能の完成速度も速くなりました。チーム全体の生産性が顕著に向上し、コードが個人的な失敗と見なされるのではなく、改善のための意見がもたらされることに誰もが快適さを感じました。

一般的なコードレビューの種類

  • オーバーザショルダーレビュー： あなたと同僚が隣り合って座り、一緒にコードを確認します。この方法は迅速で直接的ですが、フィードバックを追跡しにくく、リモートワークチームには適していません。

  • メールパスアラウンド： コードがメールで送信されます。この方法はかなり古風で、大きな変更や多くの人がレビューする必要がある場合には効果的ではありません。

  • ツール支援レビュー（プルリクエスト/マージリクエスト）： これは現在最も一般的な方法です。GitHub, GitLab, Bitbucketのようなソースコード管理ツールを使用すると、すべての変更が「プルリクエスト」（PR）または「マージリクエスト」（MR）にまとめられ、議論や追跡が非常に便利になり、明確な履歴が残ります。

効果的なコードレビュープロセス：AからZまで

コードレビュープロセスを真に効果的にするには、PR作成者（author）とレビュー担当者（reviewer）の両方が責任を持ち、正しい手順を実行する必要があります。

1. プルリクエスト（PR）作成者：入念な準備

他の人にコードをレビューしてもらう前に、これらのことをしっかりと行っているか確認してください：

  • 明確なPR説明の作成： これはレビュー担当者への「概要」です。私は常に詳細に書くように努めています：

    • この変更の目的は何ですか？（バグ修正、新機能追加、リファクタリング？）
    • それはどのような問題を解決しますか？
    • コードの主な変更点は何ですか？
    • 特にレビューが必要な箇所はありますか？
    • レビュー担当者にコードを実行してもらう必要がありますか？もしそうなら、実行方法を指示してください。

  • 送信前のコード確認（セルフレビュー）： 自分でコードを再確認します。他の人が見る前に、自分で多くの小さなエラーを見つけて修正することができます。これは双方の時間節約になるだけでなく、自分のコードを自己評価するスキルを向上させるのにも役立ちます。

  • ユニット/結合テストの実行： すべてのテストがパスしていることを確認します。新機能を追加したりバグを修正したりした場合は、それに対するテストも追加で記述してください。テストのないコードは、多くのリスクを潜在的に含んでいます。

例：効果的なPR説明

# PR名: Feat: カテゴリー別に商品リストを表示する機能を追加

**目的:**
カテゴリーIDに基づいて商品リストを取得するためのAPIエンドポイント `/api/products?category_id={id}` を追加します。
フロントエンドはこのAPIを使用して、ユーザーが特定のカテゴリーを選択したときに商品を表示します。

**解決する問題:**
これまでカテゴリーによる商品フィルタリングの方法がなく、種類別の商品表示管理が困難でした。

**主な変更点:**
- `src/controllers/productController.js`: リクエストを処理するための `getProductsByCategory` 関数を追加しました。
- `src/routes/productRoutes.js`: `category_id` クエリパラメータを受け入れるようにルート `GET /api/products` を更新しました。
- `src/services/productService.js`: `category_id` に基づいて商品をフィルタリングするためのデータベースクエリロジックを追加しました。

**レビュー担当者への注意:**
- `productService.js` 内のフィルタリングロジック、特に `category_id` が無効な場合や渡されない場合の処理を詳細に確認してください。
- 正確性を確保するために、`tests/unit/productService.test.js` に `getProductsByCategory` のユニットテストを作成済みです。

**テスト手順:**
1. 最新のリポジトリをクローンします。
2. `npm install` と `npm start` を実行してアプリケーションを起動します。
3. Postmanを開くか、`curl` を使用してAPIをテストします：
   - カテゴリーID 1で商品を取得: `curl -X GET "http://localhost:3000/api/products?category_id=1"`
   - すべての商品を取得 (category_idがない場合): `curl -X GET "http://localhost:3000/api/products"`

例：Python/Node.jsでのテスト実行

あなたのコードが既存のものを壊さないことを確認するために、テストを実行してください：

# pytestを使用するPythonプロジェクトの場合
pytest tests/

# Jestなどを使用するJavaScript/TypeScriptプロジェクトの場合
npm test
# または
yarn test

2. レビュー担当者：集中と建設的なフィードバック

レビュー担当者である場合、目標はコードをより良くすることであり、個人を批判することではないことを忘れないでください。すべてのフィードバックは品質改善を目指すべきです。

  • 変更のコンテキストを理解する： PRの説明を注意深く読み、PR作成者が提示した目的と解決策を理解するのに時間をかけます。必要であれば、直接質問して明確にすることをためらわないでください。

  • 重要な側面を確認する：

    • ロジック： コードは期待通りに動作しますか？すべてのエッジケース（例：空データ、負の値、接続エラーなど）が処理されていますか？

    • 複雑性： コードは複雑すぎず、理解しにくくないですか？関数を分割したり、より最適化されたアルゴリズムを使用したりして、簡素化できますか？

    • パフォーマンス： この変更はシステムパフォーマンスに悪影響を与えませんか？（例：過度なネストされたループ、非効率なデータベースクエリ）

    • セキュリティ： 新しいセキュリティ上の脆弱性はありませんか？（例：SQLインジェクション、XSS、機密情報の漏洩）

    • 可読性と保守性（Readability & Maintainability）： 変数名、関数名は明確で理解しやすいですか？複雑な箇所にコメントはありますか？コードはチームのコーディングスタイルに従っていますか？読みやすいコードは、後で保守や拡張が容易になります。

    • テスト： 新しいテストは十分なカバレッジをカバーしていますか？既存のテストは変更後もパスしますか？

  • 具体的かつ建設的なフィードバック：

    • 変更を提案する理由を常に説明してください。理由なしに問題点を指摘しても、説得力に欠けます。
    • 「ここは間違っている」とだけ言うのではなく、解決策を提案してください。もし自分がそのコードを書くとしたらどうするかを考えてください。
    • 友好的で、励ますような口調を使用してください。批判的な言葉は避けてください。
    • フィードバックを分類します：「Blocking」（マージする前に修正が必要、通常は深刻なエラー、バグ）と「Suggestion」（改善できるが必須ではない、通常はスタイルや小さなパフォーマンスに関するもの）。

例：効果的なレビューコメント

「このコードはひどい」とか「これは間違っている」とだけ言うのではなく、具体的に指摘し、提案してください：

// 変更前:
- def get_active_users():
-     users = database.query("SELECT * FROM users WHERE status = 'active'")
-     return users

// 変更後 (レビュー担当者のコメント):
+ def get_active_users():
+     """
+     アクティブなユーザーのリストをすべて返します。
+ 
+     提案 (ブロック対象): SQL文字列を直接連結しています。入力データが適切にサニタイズされていない場合、SQLインジェクション脆弱性につながる可能性があります。
+     より安全なデータベースクエリのために、ORM (Python用SQLAlchemyなど) またはプリペアドステートメントの使用をお勧めします。
+ 
+     提案 (推奨): また、ユーザーリストが大きくなる可能性がある場合、データロードのパフォーマンスを向上させるためにページネーションの追加を検討すべきだと思います。
+     """
+     # プリペアドステートメントの使用例 (仮定的な使用法)
+     # users = database.execute("SELECT * FROM users WHERE status = ?", ('active',))
+     # ORMの使用例 (Django ORMまたはSQLAlchemyなど)
+     # users = User.objects.filter(status='active').all()
+     users = database.query("SELECT * FROM users WHERE status = 'active'")
+     return users

このコメントは、問題点（SQLインジェクション、ページネーションの欠如）を指摘するだけでなく、具体的な解決策（ORM/プリペアドステートメント）を提案し、その利点（セキュリティ、パフォーマンス）を説明し、フィードバックを明確に分類しています。

3. 議論と完成

  • 双方向の対話： PR作成者はすべてのフィードバックを受け入れる必要はありません。コードをそのまま維持する正当な理由がある場合は、明確に説明してください。目標は、誰が正しいか間違っているかではなく、コードにとって最適な解決策を見つけることです。

  • PR作成者によるコードの更新： フィードバックに基づいて、PR作成者はコードを修正し、更新します。その後、コメントを解決済みとしてマークするか、レビュー担当者に新しい変更を確認するよう求めます。

  • レビュー担当者による承認（approve）または追加変更要求： すべての問題が適切に解決されたら、レビュー担当者はPRを承認します。まだ修正が必要な点がある場合は、コードが望ましい品質に達するまで変更を要求し続けます。

あなたのコードレビュースキルを向上させる

コードレビューは他人のコードを見るだけではありません。チーム全体が共に成長し、自分自身のスキルを向上させるための素晴らしい機会です。

サポートツールの活用

すべてを手動で行おうとしないでください。これらのツールは、繰り返し行うタスクを自動化し、大いに役立ちます。

  • リンターとフォーマッター：

    • リンター（例：JavaScript用ESLint、Python用Pylint）は、構文エラー、コーディングスタイルに関する問題、および潜在的なバグの検出を助けます。これらは、コードをよりクリーンにするための厳しい先生のようなものです。

    • フォーマッター（例：JavaScript用Prettier、Python用Black）は、特定のルールセットに従ってコードを自動的にフォーマットし、コードを常にきれいで一貫性のあるものに保ち、スペースや改行について心配する必要がなくなります。

    私は常に、コミットする前にチームにリンター/フォーマッターを自動的に実行することを推奨しています。これにより、レビュー前にスタイルの問題の80%が解決され、ロジックに集中できるようになります。

    例：リンター/フォーマッターの実行

    # 現在のディレクトリのPythonコードをBlackでフォーマット
black .

# ESLintを実行してJavaScript/TypeScriptのスタイルエラーをチェックし自動修正
eslint --fix src/

  • 静的解析ツール： これらのツール（例：SonarQube、Python用Bandit）はさらに深く掘り下げ、リンターでは見つけられないセキュリティ脆弱性、過度な複雑性、潜在的なアーキテクチャ上の問題を検索します。これらは、ソースコードの品質とリスクについてより包括的な視点を提供します。

  • CI/CD（継続的インテグレーション/継続的デリバリー）への統合： CI/CDプロセスでリンター、フォーマッター、およびテストを自動的に実行します。エラーがある場合、PRはマージされず、開発者は事前に修正する必要があります。これにより、基本的なエラーが自動的に捕捉されるため、レビュー時間を大幅に節約できます。

細部ではなく、価値に焦点を当てる

レビュー時には、プロジェクトとユーザーに大きな影響を与える主要な問題に優先順位を付けてください。

  • ビジネスロジックの誤り。
  • 深刻なセキュリティ脆弱性。
  • ユーザー体験に大きな影響を与えるパフォーマンスの問題。
  • 将来の拡張を困難にする可能性のある大規模なアーキテクチャの変更。
  • 特定の状況でシステムを簡単にクラッシュさせるコード。

余分なセミコロン、最適ではないが理解しやすい変数名、誤ったスペースなどの細部は、提案（suggestion）として指摘されることはありますが、PRをブロックする理由にはすべきではありません。これらの問題の処理は、前述の自動化ツールに任せましょう。

すべてのレビューから学ぶ

  • 他人のコードをレビューするとき： さまざまな問題解決の方法に出会うでしょう。彼らがなぜそのようにしたのか、彼らの解決策が自分の方法よりも優れているか、そしてそこから何を学べるかを考えてください。

  • 自分のコードがレビューされるとき： それを個人的な批判と見なさないでください。学び、改善する機会だと考えてください。良いフィードバックを覚えておき、次回以降に適用することで、日々より良い開発者になることができます。

友好的で効果的なコードレビューのための実践的なヒント

  • PRをコンパクトに保つ： 1つのPRは、1つの具体的な問題を解決する1つの変更のみを含むべきです。5000行のコード変更を含むPRをレビューするのは非常に苦痛であり、エラーを見落としやすいです。理想的な目標は、200〜300行以下のコード変更のPRです。

  • 長時間のレビューを避ける： 人間の脳の集中力には限界があります。30〜60分程度のレビューを目指してください。PRが大きすぎる場合は、PR作成者に分割を提案するか、複数回に分けてレビューしてください。

  • ポジティブな雰囲気を作る： レビューを開始する際に、もし良い点があれば褒め言葉から始めてください。「この認証部分の処理、とてもクリーンで分かりやすくて良いですね！」と言うと、PR作成者は他のフィードバックを受け入れやすくなります。

  • 絵文字を使用する： 時には、絵文字が言葉よりも感情をうまく伝えることがあります。例：

    • `👍`：良いコードや同意するとき。
    • `🤔`：考えている、またはまだ問題を理解していないとき。
    • `😅`：ちょっとした可愛いエラーや面白いことを見つけたとき。

  • 質問することをためらわない： コードの一部やその目的が理解できない場合は、すぐに質問してください。すべてを知っていることを誰も期待していません。質問することは、PR作成者が自分のコードが他の人にとって十分明確で理解しやすいかどうかを再考するのにも役立ちます。

  • 必要であれば直接会う： 複雑な問題や、コメントでの議論が膠着状態に陥った場合は、短時間のミーティングや「オーバーザショルダーレビュー」を積極的に提案して、より迅速に解決し、誤解を避けてください。

結論

コードレビューは単なる技術的なプロセスではなく、現代のソフトウェア開発において重要な文化です。それは、チームが製品の品質を向上させ、個人のスキルを継続的に発展させるための架け橋となります。

効果的なコードレビュープロセスを実践することで、あなたはより良いソースコードを書くだけでなく、批判的思考力と高い協力能力を持つ、より優れた開発者になるでしょう。今日からすぐに適用を開始し、コードレビューをあなたのキャリアパスにおける不可欠なツールとして捉えてください！

Share:
Tags: