Flutter Dart Code Review

Flutter/Dartコードレビューベストプラクティス

Flutter/Dartアプリケーションをレビューするための包括的なライブラリに依存しないチェックリスト。これらの原則は、どの状態管理ソリューション、ルーティングライブラリ、またはDIフレームワークを使用していても適用されます。

1. 全般的なプロジェクトの健全性

• [ ] プロジェクトは一貫したフォルダー構造に従っている（フィーチャーファーストまたはレイヤーファースト）
• [ ] 適切な関心の分離: UI、ビジネスロジック、データレイヤー
• [ ] ウィジェットにビジネスロジックがない; ウィジェットは純粋にプレゼンテーション
• [ ] pubspec.yamlが整理されている — 未使用の依存関係がなく、バージョンが適切に固定されている
• [ ] analysis_options.yamlに厳格なリントセットと厳格なアナライザー設定が含まれている
• [ ] 本番コードにprint()文がない — dart:developerのlog()またはロギングパッケージを使用
• [ ] 生成されたファイル（.g.dart、.freezed.dart、.gr.dart）が最新か.gitignoreに含まれている
• [ ] プラットフォーム固有のコードが抽象化の背後に分離されている

2. Dart言語の落とし穴

• [ ] 暗黙的なdynamic: 型アノテーションの欠如がdynamicにつながる — strict-casts、strict-inference、strict-raw-typesを有効にする
• [ ] Null安全の誤用: 適切なnullチェックやDart 3のパターンマッチング（if (value case var v?)）の代わりに過度な!（bang演算子）
• [ ] 型プロモーションの失敗: ローカル変数プロモーションが機能する場所でthis.fieldを使用
• [ ] 過度に広い例外のキャッチ: on句なしのcatch (e); 常に例外型を指定する
• [ ] `Error`のキャッチ: Errorのサブタイプはバグを示し、キャッチすべきでない
• [ ] 未使用の`async`: awaitしないasyncマークされた関数 — 不要なオーバーヘッド
• [ ] `late`の過剰使用: nullable型やコンストラクターの初期化がより安全な場所でのlateの使用; エラーをランタイムに先送りにする
• [ ] ループでの文字列連結: 繰り返しの文字列構築には+の代わりにStringBufferを使用
• [ ] `const`コンテキストでの可変状態: constコンストラクタークラスのフィールドは可変であるべきでない
• [ ] `Future`の戻り値の無視: 意図を示すためにawaitを使用するか明示的にunawaited()を呼び出す
• [ ] `final`が使える場所での`var`: ローカル変数にはfinalを、コンパイル時定数にはconstを優先
• [ ] 相対インポート: 一貫性のためにpackage:インポートを使用
• [ ] 公開された可変コレクション: パブリックAPIは生のList/Mapではなく変更不可能なビューを返すべき
• [ ] Dart 3パターンマッチングの欠如: 冗長なisチェックと手動キャストの代わりにswitch式とif-caseを優先
• [ ] 複数の戻り値のための使い捨てクラス: 単一使用のDTOの代わりにDart 3のレコード(String, int)を使用
• [ ] 本番コードでの`print()`: dart:developerのlog()またはプロジェクトのロギングパッケージを使用; print()はログレベルがなくフィルタリングできない

3. ウィジェットのベストプラクティス

ウィジェットの分解:

• [ ] build()メソッドが約80-100行を超える単一ウィジェットがない
• [ ] ウィジェットがカプセル化と変化の仕方（再構築の境界）によって分割されている
• [ ] ウィジェットを返すプライベートな_build*()ヘルパーメソッドが別のウィジェットクラスに抽出されている（要素の再利用、const伝播、フレームワーク最適化を可能にする）
• [ ] 可変のローカル状態が必要でない場合、Statelessウィジェットが優先される
• [ ] 抽出されたウィジェットが再利用可能な場合、別のファイルに存在する

Constの使用:

• [ ] constコンストラクターを可能な限り使用 — 不要な再構築を防ぐ
• [ ] 変化しないコレクションにconstリテラルを使用（const []、const {}）
• [ ] すべてのフィールドがfinalの場合、コンストラクターがconstとして宣言されている

Keyの使用:

• [ ] 並べ替え時に状態を保持するためにValueKeyをリスト/グリッドで使用
• [ ] GlobalKeyは控えめに使用 — ツリー全体の状態アクセスが本当に必要な場合のみ
• [ ] UniqueKeyをbuild()内で使用しない — フレームごとに再構築を強制する
• [ ] 単一の値ではなくデータオブジェクトのアイデンティティに基づく場合はObjectKeyを使用

テーマとデザインシステム:

• [ ] 色はTheme.of(context).colorSchemeから取得 — Colors.redやhex値のハードコードなし
• [ ] テキストスタイルはTheme.of(context).textThemeから取得 — 生のフォントサイズのインラインTextStyleなし
• [ ] ダークモードの互換性を確認 — 明るい背景についての仮定なし
• [ ] スペーシングとサイジングは一貫したデザイントークンまたは定数を使用し、マジックナンバーではない

buildメソッドの複雑さ:

• [ ] build()内にネットワーク呼び出し、ファイルI/O、または重い計算がない
• [ ] build()内にFuture.then()またはasync作業がない
• [ ] build()内にサブスクリプション作成（.listen()）がない
• [ ] setState()が可能な限り小さいサブツリーに限定されている

4. 状態管理（ライブラリに依存しない）

これらの原則はすべてのFlutter状態管理ソリューション（BLoC、Riverpod、Provider、GetX、MobX、Signals、ValueNotifier など）に適用されます。

アーキテクチャ:

• [ ] ビジネスロジックがウィジェットレイヤーの外にある — 状態管理コンポーネント（BLoC、Notifier、Controller、Store、ViewModelなど）内
• [ ] 状態マネージャーが依存関係をインジェクションで受け取り、内部で構築しない
• [ ] サービスまたはリポジトリレイヤーがデータソースを抽象化 — ウィジェットと状態マネージャーはAPIやデータベースを直接呼び出すべきでない
• [ ] 状態マネージャーが単一の責務を持つ — 無関係な懸念を処理する「god」マネージャーなし
• [ ] コンポーネント間の依存関係がソリューションの規約に従う:
• Riverpodでは: プロバイダーがref.watchを通じて他のプロバイダーに依存することは予期されている — 循環または過度に絡み合ったチェーンのみフラグを立てる
• BLoCでは: BLoCが他のBLoCに直接依存すべきでない — 共有リポジトリまたはプレゼンテーション層の調整を優先する
• 他のソリューションでは: コンポーネント間通信の文書化された規約に従う

イミュータビリティと値の等値性（イミュータブル状態ソリューション用: BLoC、Riverpod、Redux）:

• [ ] 状態オブジェクトがイミュータブル — インプレースで変異させるのではなく、copyWith()またはコンストラクターで新しいインスタンスを作成
• [ ] 状態クラスが==とhashCodeを適切に実装（すべてのフィールドが比較に含まれる）
• [ ] メカニズムがプロジェクト全体で一貫 — 手動オーバーライド、Equatable、freezed、Dartレコード、またはその他
• [ ] 状態オブジェクト内のコレクションが生の可変List/Mapとして公開されていない

リアクティビティの規律（リアクティブ変異ソリューション用: MobX、GetX、Signals）:

• [ ] 状態がソリューションのリアクティブAPI（MobXでの@action、signalでの.value、GetXでの.obs）を通じてのみ変異される — 直接フィールド変異は変更追跡をバイパスする
• [ ] 派生値がソリューションの計算メカニズムを使用し、冗長に保存されない
• [ ] リアクションとディスポーザーが適切にクリーンアップされる（MobXでのReactionDisposer、Signalsでのeffectクリーンアップ）

状態の形状設計:

• [ ] 相互に排他的な状態がsealed型、ユニオン変体、またはソリューションの組み込み非同期状態型（例: RiverpodのAsyncValue）を使用 — ブールフラグ（isLoading、isError、hasData）は使わない
• [ ] すべての非同期操作がローディング、成功、エラーを異なる状態としてモデル化
• [ ] すべての状態変体がUIで網羅的に処理 — サイレントに無視されるケースなし
• [ ] エラー状態が表示のためのエラー情報を持つ; ローディング状態は古いデータを持たない
• [ ] 可変のデータがローディングインジケーターとして使用されない — 状態は明示的

// 悪い例 — ブールフラグの混乱が不可能な状態を許可する
class UserState {
  bool isLoading = false;
  bool hasError = false; // isLoading && hasErrorが表現可能！
  User? user;
}

// 良い例（イミュータブルアプローチ） — sealed型が不可能な状態を表現不可能にする
sealed class UserState {}
class UserInitial extends UserState {}
class UserLoading extends UserState {}
class UserLoaded extends UserState {
  final User user;
  const UserLoaded(this.user);
}
class UserError extends UserState {
  final String message;
  const UserError(this.message);
}

// 良い例（リアクティブアプローチ） — observableのenum + データ、リアクティビティAPIを通じた変異
// enum UserStatus { initial, loading, loaded, error }
// ソリューションのobservable/signalを使用してstatusとdataを別々にラップする

再構築の最適化:

• [ ] 状態コンシューマーウィジェット（Builder、Consumer、Observer、Obx、Watchなど）をできるだけ狭くスコープする
• [ ] 特定のフィールドが変化した場合のみ再構築するためにセレクターを使用 — すべての状態エミッションで再構築しない
• [ ] ツリーを通じた再構築の伝播を止めるためにconstウィジェットを使用
• [ ] 計算/派生状態がリアクティブに計算され、冗長に保存されない

サブスクリプションと廃棄:

• [ ] すべての手動サブスクリプション（.listen()）がdispose() / close()でキャンセルされる
• [ ] ストリームコントローラーが不要になったら閉じられる
• [ ] タイマーが廃棄ライフサイクルでキャンセルされる
• [ ] フレームワーク管理のライフサイクルが手動サブスクリプションより優先される（.listen()よりも宣言的ビルダー）
• [ ] 非同期コールバックでのsetState前にmountedチェック
• [ ] await後にBuildContextをcontext.mountedをチェックせずに使用しない（Flutter 3.7+） — 古いコンテキストはクラッシュを引き起こす
• [ ] 非同期ギャップの後にウィジェットがまだマウントされていることを確認せずにナビゲーション、ダイアログ、またはscaffoldメッセージを使用しない
• [ ] BuildContextをシングルトン、状態マネージャー、または静的フィールドに保存しない

ローカル対グローバル状態:

• [ ] 一時的なUI状態（チェックボックス、スライダー、アニメーション）がローカル状態（setState、ValueNotifier）を使用
• [ ] 共有状態が必要な分だけリフトされる — 過度にグローバル化されない
• [ ] フィーチャースコープの状態がフィーチャーがアクティブでなくなったときに適切に廃棄される

5. パフォーマンス

不要な再構築:

• [ ] setState()がルートウィジェットレベルで呼び出されない — 状態変更をローカル化する
• [ ] constウィジェットが再構築の伝播を止めるために使用される
• [ ] RepaintBoundaryが独立して再描画する複雑なサブツリーの周りに使用される
• [ ] AnimatedBuilderのchildパラメーターがアニメーションから独立したサブツリーに使用される

build()内の高コスト操作:

• [ ] build()内で大きなコレクションのソート、フィルタリング、マッピングがない — 状態管理レイヤーで計算する
• [ ] build()内でregexのコンパイルがない
• [ ] MediaQuery.of(context)の使用が具体的（例: MediaQuery.sizeOf(context)）

画像の最適化:

• [ ] ネットワーク画像がキャッシングを使用（プロジェクトに適したキャッシングソリューション）
• [ ] ターゲットデバイスに適した画像解像度（サムネイルに4K画像をロードしない）
• [ ] Image.assetとcacheWidth/cacheHeightを使用して表示サイズでデコードする
• [ ] ネットワーク画像にプレースホルダーとエラーウィジェットが提供されている

遅延ローディング:

• [ ] 大きなまたは動的なリストにはListView(children: [...])の代わりにListView.builder / GridView.builderを使用（小さくて静的なリストにはコンクリートコンストラクターが適切）
• [ ] 大きなデータセットにページネーションが実装されている
• [ ] Webビルドで重いライブラリに遅延ローディング（deferred as）を使用

その他:

• [ ] アニメーションでOpacityウィジェットを避ける — AnimatedOpacityまたはFadeTransitionを使用
• [ ] アニメーションでクリッピングを避ける — 画像を事前にクリップする
• [ ] ウィジェットでoperator ==をオーバーライドしない — 代わりにconstコンストラクターを使用
• [ ] 組み込み次元ウィジェット（IntrinsicHeight、IntrinsicWidth）を控えめに使用（追加のレイアウトパス）

6. テスト

テストの種類と期待値:

• [ ] ユニットテスト: すべてのビジネスロジック（状態マネージャー、リポジトリ、ユーティリティ関数）をカバー
• [ ] ウィジェットテスト: 個々のウィジェットの動作、インタラクション、視覚的出力をカバー
• [ ] 統合テスト: 重要なユーザーフローをエンドツーエンドでカバー
• [ ] ゴールデンテスト: デザインクリティカルなUIコンポーネントのピクセル単位の比較

カバレッジの目標:

• [ ] ビジネスロジックで80%以上のライン カバレッジを目指す
• [ ] すべての状態遷移が対応するテストを持つ（ローディング→成功、ローディング→エラー、リトライなど）
• [ ] エッジケースのテスト: 空の状態、エラー状態、ローディング状態、境界値

テストの分離:

• [ ] 外部依存関係（APIクライアント、データベース、サービス）がモック化またはフェイク化されている
• [ ] 各テストファイルが正確に1つのクラス/ユニットをテストする
• [ ] テストが実装の詳細ではなく動作を検証する
• [ ] スタブが各テストに必要な動作のみを定義する（最小限のスタッビング）
• [ ] テストケース間で共有された可変状態がない

ウィジェットテストの品質:

• [ ] pumpWidgetとpumpが非同期操作に対して正しく使用されている
• [ ] find.byType、find.text、find.byKeyが適切に使用されている
• [ ] タイミングに依存する不安定なテストがない — pumpAndSettleまたは明示的なpump(Duration)を使用