affaan-m/everything-claude-code/flutter-dart-code-review

Flutter/Dart Code Review Best Practices

Comprehensive, library-agnostic checklist for reviewing Flutter/Dart applications. These principles apply regardless of which state management solution, routing library, or DI framework is used.

1. General Project Health

• [ ] Project follows consistent folder structure (feature-first or layer-first)
• [ ] Proper separation of concerns: UI, business logic, data layers
• [ ] No business logic in widgets; widgets are purely presentational
• [ ] pubspec.yaml is clean — no unused dependencies, versions pinned appropriately
• [ ] analysis_options.yaml includes a strict lint set with strict analyzer settings enabled
• [ ] No print() statements in production code — use dart:developer log() or a logging package
• [ ] Generated files (.g.dart, .freezed.dart, .gr.dart) are up-to-date or in .gitignore
• [ ] Platform-specific code isolated behind abstractions

2. Dart Language Pitfalls

• [ ] Implicit dynamic: Missing type annotations leading to dynamic — enable strict-casts, strict-inference, strict-raw-types
• [ ] Null safety misuse: Excessive ! (bang operator) instead of proper null checks or Dart 3 pattern matching (if (value case var v?))
• [ ] Type promotion failures: Using this.field where local variable promotion would work
• [ ] Catching too broadly: catch (e) without on clause; always specify exception types
• [ ] Catching `Error`: Error subtypes indicate bugs and should not be caught
• [ ] Unused `async`: Functions marked async that never await — unnecessary overhead
• [ ] `late` overuse: late used where nullable or constructor initialization would be safer; defers errors to runtime
• [ ] String concatenation in loops: Use StringBuffer instead of + for iterative string building
• [ ] Mutable state in `const` contexts: Fields in const constructor classes should not be mutable
• [ ] Ignoring `Future` return values: Use await or explicitly call unawaited() to signal intent
• [ ] `var` where `final` works: Prefer final for locals and const for compile-time constants
• [ ] Relative imports: Use package: imports for consistency
• [ ] Mutable collections exposed: Public APIs should return unmodifiable views, not raw List/Map
• [ ] Missing Dart 3 pattern matching: Prefer switch expressions and if-case over verbose is checks and manual casting
• [ ] Throwaway classes for multiple returns: Use Dart 3 records (String, int) instead of single-use DTOs
• [ ] `print()` in production code: Use dart:developer log() or the project's logging package; print() has no log levels and cannot be filtered

3. Widget Best Practices

Widget decomposition:

• [ ] No single widget with a build() method exceeding ~80-100 lines
• [ ] Widgets split by encapsulation AND by how they change (rebuild boundaries)
• [ ] Private _build*() helper methods that return widgets are extracted to separate widget classes (enables element reuse, const propagation, and framework optimizations)
• [ ] Stateless widgets preferred over Stateful where no mutable local state is needed
• [ ] Extracted widgets are in separate files when reusable

Const usage:

• [ ] const constructors used wherever possible — prevents unnecessary rebuilds
• [ ] const literals for collections that don't change (const [], const {})
• [ ] Constructor is declared const when all fields are final

Key usage:

• [ ] ValueKey used in lists/grids to preserve state across reorders
• [ ] GlobalKey used sparingly — only when accessing state across the tree is truly needed
• [ ] UniqueKey avoided in build() — it forces rebuild every frame
• [ ] ObjectKey used when identity is based on a data object rather than a single value

Theming & design system:

• [ ] Colors come from Theme.of(context).colorScheme — no hardcoded Colors.red or hex values
• [ ] Text styles come from Theme.of(context).textTheme — no inline TextStyle with raw font sizes
• [ ] Dark mode compatibility verified — no assumptions about light background
• [ ] Spacing and sizing use consistent design tokens or constants, not magic numbers

Build method complexity:

• [ ] No network calls, file I/O, or heavy computation in build()
• [ ] No Future.then() or async work in build()
• [ ] No subscription creation (.listen()) in build()
• [ ] setState() localized to smallest possible subtree

4. State Management (Library-Agnostic)

These principles apply to all Flutter state management solutions (BLoC, Riverpod, Provider, GetX, MobX, Signals, ValueNotifier, etc.).

Architecture:

• [ ] Business logic lives outside the widget layer — in a state management component (BLoC, Notifier, Controller, Store, ViewModel, etc.)
• [ ] State managers receive dependencies via injection, not by constructing them internally
• [ ] A service or repository layer abstracts data sources — widgets and state managers should not call APIs or databases directly
• [ ] State managers have a single responsibility — no "god" managers handling unrelated concerns
• [ ] Cross-component dependencies follow the solution's conventions:
• In Riverpod: providers depending on providers via ref.watch is expected — flag only circular or overly tangled chains
• In BLoC: blocs should not directly depend on other blocs — prefer shared repositories or presentation-layer coordination
• In other solutions: follow the documented conventions for inter-component communication

Immutability & value equality (for immutable-state solutions: BLoC, Riverpod, Redux):

• [ ] State objects are immutable — new instances created via copyWith() or constructors, never mutated in-place
• [ ] State classes implement == and hashCode properly (all fields included in comparison)
• [ ] Mechanism is consistent across the project — manual override, Equatable, freezed, Dart records, or other
• [ ] Collections inside state objects are not exposed as raw mutable List/Map

Reactivity discipline (for reactive-mutation solutions: MobX, GetX, Signals):

• [ ] State is only mutated through the solution's reactive API (@action in MobX, .value on signals, .obs in GetX) — direct field mutation bypasses change tracking
• [ ] Derived values use the solution's computed mechanism rather than being stored redundantly
• [ ] Reactions and disposers are properly cleaned up (ReactionDisposer in MobX, effect cleanup in Signals)

State shape design:

• [ ] Mutually exclusive states use sealed types, union variants, or the solution's built-in async state type (e.g. Riverpod's AsyncValue) — not boolean flags (isLoading, isError, hasData)
• [ ] Every async operation models loading, success, and error as distinct states
• [ ] All state variants are handled exhaustively in UI — no silently ignored cases
• [ ] Error states carry error information for display; loading states don't carry stale data
• [ ] Nullable data is not used as a loading indicator — states are explicit

// BAD — boolean flag soup allows impossible states
class UserState {
  bool isLoading = false;
  bool hasError = false; // isLoading && hasError is representable!
  User? user;
}
 
// GOOD (immutable approach) — sealed types make impossible states unrepresentable
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);
}
 
// GOOD (reactive approach) — observable enum + data, mutations via reactivity API
// enum UserStatus { initial, loading, loaded, error }
// Use your solution's observable/signal to wrap status and data separately

Rebuild optimization:

• [ ] State consumer widgets (Builder, Consumer, Observer, Obx, Watch, etc.) scoped as narrow as possible
• [ ] Selectors used to rebuild only when specific fields change — not on every state emission
• [ ] const widgets used to stop rebuild propagation through the tree
• [ ] Computed/derived state is calculated reactively, not stored redundantly

Subscriptions & disposal:

• [ ] All manual subscriptions (.listen()) are cancelled in dispose() / close()
• [ ] Stream controllers are closed when no longer needed
• [ ] Timers are cancelled in disposal lifecycle
• [ ] Framework-managed lifecycle is preferred over manual subscription (declarative builders over .listen())
• [ ] mounted check before setState in async callbacks
• [ ] BuildContext not used after await without checking context.mounted (Flutter 3.7+) — stale context causes crashes
• [ ] No navigation, dialogs, or scaffold messages after async gaps without verifying the widget is still mounted
• [ ] BuildContext never stored in singletons, state managers, or static fields

Local vs global state:

• [ ] Ephemeral UI state (checkbox, slider, animation) uses local state (setState, ValueNotifier)
• [ ] Shared state is lifted only as high as needed — not over-globalized
• [ ] Feature-scoped state is properly disposed when the feature is no longer active

5. Performance

Unnecessary rebuilds:

• [ ] setState() not called at root widget level — localize state changes
• [ ] const widgets used to stop rebuild propagation
• [ ] RepaintBoundary used around complex subtrees that repaint independently
• [ ] AnimatedBuilder child parameter used for subtrees independent of animation

Expensive operations in build():

• [ ] No sorting, filtering, or mapping large collections in build() — compute in state management layer
• [ ] No regex compilation in build()
• [ ] MediaQuery.of(context) usage is specific (e.g., MediaQuery.sizeOf(context))

Image optimization:

• [ ] Network images use caching (any caching solution appropriate for the project)
• [ ] Appropriate image resolution for target device (no loading 4K images for thumbnails)
• [ ] Image.asset with cacheWidth/cacheHeight to decode at display size
• [ ] Placeholder and error widgets provided for network images

Lazy loading:

• [ ] ListView.builder / GridView.builder used instead of ListView(children: [...]) for large or dynamic lists (concrete constructors are fine for small, static lists)
• [ ] Pagination implemented for large data sets
• [ ] Deferred loading (deferred as) used for heavy libraries in web builds

Other:

• [ ] Opacity widget avoided in animations — use AnimatedOpacity or FadeTransition
• [ ] Clipping avoided in animations — pre-clip images
• [ ] operator == not overridden on widgets — use const constructors instead
• [ ] Intrinsic dimension widgets (IntrinsicHeight, IntrinsicWidth) used sparingly (extra layout pass)

6. Testing

Test types and expectations:

• [ ] Unit tests: Cover all business logic (state managers, repositories, utility functions)
• [ ] Widget tests: Cover individual widget behavior, interactions, and visual output
• [ ] Integration tests: Cover critical user flows end-to-end
• [ ] Golden tests: Pixel-perfect comparisons for design-critical UI components

Coverage targets:

• [ ] Aim for 80%+ line coverage on business logic
• [ ] All state transitions have corresponding tests (loading → success, loading → error, retry, etc.)
• [ ] Edge cases tested: empty states, error states, loading states, boundary values

Test isolation:

• [ ] External dependencies (API clients, databases, services) are mocked or faked
• [ ] Each test file tests exactly one class/unit
• [ ] Tests verify behavior, not implementation details
• [ ] Stubs define only the behavior needed for each test (minimal stubbing)
• [ ] No shared mutable state between test cases