code-refactor-review

コードリファクタリングのレビュー

コードの変更を、再利用性、コンポジション、コードベースとの一貫性、そして無駄なコードに見えるものがないかという観点から、徹底的にレビューします。

最初の確認

1. 差分全体を確認します。
   • ステージングされていない変更には git diff を使います。
   • ステージング済みの変更がある場合は git diff HEAD を使います。
   • PR の URL がある場合は、レビューの詳細に入る前に、変更されたファイルを確認し、機能の流れを把握します。
2. 必要に応じて、呼び出しスタックやデータフローをたどります。機能がどのように接続されているかを理解せず、孤立した行だけをレビューしてはいけません。
3. 新しいヘルパー、コンポーネント、フック、パターンを評価する前に、コードベースを検索します。新しい抽象化を考案するより、近くの実装や同種の実装を優先します。

レビューの観点

1. 既存コードの再利用

• 新たに書かれたコードを受け入れる前に、既存のユーティリティ、コンポーネント、フック、サーバーアクション、ルートのパターン、コピーのパターン、スタイルのプリミティブがないか確認します。
• ロジックの重複、コピーされたヘルパー、またはコードベースにすでにある機能の独自実装を指摘します。
• 少し拡張が必要でも、既存のフローの再利用を優先します。
• 新しい共有ヘルパーが作られている場合は、実際に再利用されているか、曖昧な名前を付けたプライベートロジックの切り出しにすぎないかを確認します。

2. コードベースとの一貫性

• ファイルの配置は、ドメインや近隣の機能に沿っている必要があります。無作為にトップレベルの lib に集められている場合は注意します。
• 名前はコードが実際に行うことを表し、同種のファイルや関数の命名に沿っている必要があります。
• 実際の製品や API の区別でない限り、実装の詳細を名前に含めないようにします。
• 既存の結果、エラー、読み込みのパターンに合わせます。コードベースに標準の型がある場合、独自の成功・失敗型を考案してはいけません。
• ユーザー向けテキストは、既存の文体やトーンに合わせます。

3. コンポジションと境界

• 関数は適切な抽象化レベルで、1つのことを行う必要があります。
• フラグ、API 呼び出し、変換、UI の状態、ログ記録、スケジューリングなど、無関係な関心事を混ぜた寄せ集めのモジュールは避けます。
• パラメーターが増えすぎないようにします。関数に多くの調整項目が必要なら、境界が適切かどうか確認します。
• コールバック、ラッパー、メモ化されたヘルパー、プロパティの受け渡しを連鎖させるより、シンプルなコンポジションを優先します。
• 2つの基盤エンティティを1つの製品概念として扱う場合、中間コンポーネント全体で1つの転送モデル／ビューモデルにまとめます。ブラックボックスのコンポーネントには、統一されたプロパティ／コールバックを渡し、リモートか事前構築済みかといった違いを意識させないようにします。永続化やペイロード形式で必要な場合に限り、ルートやアダプターでコアエンティティに分割し直します。
• ドメイン固有のロジックは、ドメインをまたいだ再利用が実証されていない限り、そのドメインの近くに保ちます。

4. 無駄なコードの検出

以下を指摘し、依頼された場合は削除します。

• コメントの無駄：自明なコメント、不自然なコードを正当化するコメント、明確なコードに置き換えるべき長いコメント、古くなった PR 説明の文脈。
• ヘルパーの無駄：意味を加えない小さなラッパー、1つの関数を短く見せるためだけに作られたヘルパーファイル、不要な間接化。
• 型の無駄：一度しか使わない型のエクスポート、独自の結果形式、型推論の方が明確なのに付けられた型注釈、不自然なコードを取り繕うだけの型。
• メモ化／コールバックの無駄：測定上または構造上の理由なしに追加された useMemo／useCallback。
• エフェクトの無駄：プロパティや状態を反映するだけのエフェクト、派生状態をリセットするエフェクト、イベントを後から処理するエフェクト。
• 互換性のための残骸：一貫した最終形を構築する代わりに、偶発的なアーキテクチャを維持するために付け足された動作。
• 差分の膨張：設計を改善しない、無関係な名前変更、フォーマット変更、コメント、ラッパーによって PR の変更量を増やすこと。

5. React / Next.js の品質

• 「Effect が不要な場合」を適用します。値はレンダー中に導出し、イベントによって発生する処理はイベントハンドラーに移し、適切な場合はキーを使って状態をリセットします。
• 重複する状態や同期用の Effect は避けます。
• パフォーマンスへの漠然とした不安を解消するだけのメモ化は追加しません。メモ化は、レンダー時の同一性に関する実際の問題や、コストの高い計算を解決する場合に使います。
• props やコールバックを複雑に組み合わせるより、わかりやすいコンポーネントの境界を優先します。
• サーバー／データのコードでは、不要なウォーターフォールを避け、コードベースにパターンがある場合は、独立した処理を並行して実行します。

6. 最小限にする

• 新しい構造を追加するより、コードを削除することを優先します。
• 再利用性や可読性が向上する場合を除き、細かなヘルパー関数をいくつも作るより、明確な関数を1つ使います。
• 問題に見合った範囲で修正します。
• 将来の読者の曖昧さを解消しない限り、アーキテクチャ、ドキュメント、コメントを追加しません。

出力形式

まず、判定を示します。

• clean — 問題となる懸念はありません。
• mostly clean — 軽微な整理のみが必要です。
• needs cleanup — 再利用、合成、一貫性に関する重要な問題があります。

次に、優先度順に指摘事項を列挙します。各指摘には以下を含めます。

1. ファイルパスと該当するシンボル／箇所。
2. 何が無駄、または一貫性に欠けるように見えるか。
3. 再利用すべき既存のパターンやコードが見つかった場合は、その内容。
4. 最小限の推奨修正。

ユーザーがレビューのみを依頼した場合は、ファイルを編集しないでください。修正を依頼された場合は、直接変更を行い、変更内容を要約してください。

注意すべき兆候

• 「既存のコードを再利用できるか」という問いに、検索して答えていない。
• utils、helpers、shared のような曖昧な名前や、実装固有の名前を付けた新しいトップレベルのヘルパー。
• 重要な副作用が名前からわからない関数。
• 理由もなく index.ts だけを含むディレクトリ。
• すでに別の場所でエクスポートされているものを再エクスポートしている。
• プロダクトに既存のコンポーネントやパターンがあるのに、新しい独自プリミティブを追加している。
• 扱いにくい props やフラグの理由を説明する長いコメント。
• 1つのローカル関数を支えるだけの新しい型が複数ある。
• 状態の所有権を単純化すれば不要になるコールバック、メモ化、Effect のコード。

ルール

• 率直かつ簡潔にしてください。余計な説明は避けてください。
• アーキテクチャをでっち上げないでください。コードベースのパターンに基づいて判断してください。
• 再利用可能、または一貫性がないと主張する前に検索してください。
• 変更を提案する際は、対象プロジェクトのローカルなスタイルに従ってください。
• 判断に迷う場合は、左から右へ読んで明快なコードを優先してください。