2. Code Quality & Architecture

Code Quality Metrics

Error Handling Consistency60 / 100
Code Reusability55 / 100
State Management Quality70 / 100

2.1 Code Structure & Organization

Status: ⚠️Warning

Findings:

  • Project uses GitFlow-like branching strategy with multiple long-lived branches
  • Multiple branches exist: main, develop, staging which creates complexity
  • Hard to identify pending changes and what's currently deployed
  • Multiple long-lived branches increase merge conflict risk and slow down development velocity
  • No clear single source of truth for current production code
  • Well-organized code structure with clear separation of concerns (positive aspect)
  • Routes separated from route models (MVVM-like pattern)
  • Clear separation between lib/routes/, lib/routes/models/, lib/states/, lib/singletons/, lib/widgets/
  • Database layer properly abstracted with DAOs
  • Helpers directory for reusable utility functions

Evidence:

  • Git branches: main, develop, staging branches exist
  • Multiple long-lived branches make code review and tracking difficult
  • Clear directory structure:
    • lib/routes/ - UI/Route components
    • lib/routes/models/ - ViewModels/Route models
    • lib/states/ - State management classes
    • lib/singletons/ - Global singleton instances
    • lib/database/ - Database layer with DAOs
    • lib/widgets/ - Reusable widgets
    • lib/helpers/ - Utility helper functions
    • lib/models/ - Data models
  • MVVM-like pattern with route models handling business logic

Risk Level: Medium Risk

Recommendation:

  • Migrate to trunk-based development (main/master as primary branch)
  • Use short-lived feature branches that merge directly to main
  • Implement feature flags for incomplete features instead of long-lived branches
  • Finish and merge all hanging changes from existing feature branches
  • Simplify workflow: main → feature branch → PR → main → deploy
  • Use branch protection rules to enforce code review before merging to main
  • Consider documenting the architecture pattern in README
  • Consider using dependency injection instead of singletons for better testability

2.2 Error Handling

Status: ⚠️Warning

Findings:

  • Error handling is inconsistent across routes
  • Routes use .catchError() pattern (login_route, password_reset_verify_route, password_reset_route, password_reset_ask_route, redeem_two_route)
  • Some async operations may lack proper error handling
  • Error messages hardcoded in Dutch, not internationalized
  • Generic error messages don't provide specific feedback to users
  • Error handling in Dio interceptor (token refresh) resets session but may not provide user feedback
  • Some error cases show generic dialogs without specific error information

Evidence:

  • lib/routes/login_route.dart: Uses .catchError() with generic error dialog (lines 318-330)
    • Error message: "Er ging iets mis" (hardcoded Dutch)
    • Content: "Controleer je gebruikersnaam en wachtwoord" (hardcoded Dutch)
  • lib/routes/password_reset_verify_route.dart: Uses .catchError() (line 304)
  • lib/routes/password_reset_route.dart: Uses .catchError() (line 266)
  • lib/routes/password_reset_ask_route.dart: Uses .catchError() (line 283)
  • lib/routes/redeem_two_route.dart: Uses .catchError() (line 203)
  • lib/singletons/dio_singleton.dart: Token refresh error handling (lines 121-124) only resets session, doesn't notify user
  • Error messages hardcoded: "Er ging iets mis", "Controleer je gebruikersnaam en wachtwoord"
  • No centralized error handling helper function

Risk Level: Medium Risk

Recommendation:

  • Immediate actions:
    • Create centralized error handling helper function
    • Standardize error handling across all routes (use consistent pattern)
    • Add error handling to all async operations (ensure all .then() have error handlers)
    • Internationalize error messages
  • Short-term:
    • Create error handling base class or mixin for routes
    • Add specific error types and user-friendly messages
    • Implement retry mechanisms for transient errors
    • Add error boundaries for widget tree errors
    • Improve user feedback in Dio interceptor error handling

2.3 State Management

Status: ⚠️Warning

Findings:

  • Provider pattern with ChangeNotifier implemented for state management
  • Multiple state classes follow similar patterns: SessionState, ConnectivityStatus, SavingState, UserLoyaltyState, SelectedGasStationMarkerState, MapState, NavigationMenuState, ParameterState, ExplainerState, WidgetLayoutState
  • Singleton pattern used for global state access
  • State synchronization issues: State stored in both memory and database, potential for inconsistency
  • Direct database access in some places bypasses state management layer
  • Async state initialization pattern (init() method) used in SessionState
  • State persistence to database happens on every state change (potential performance issue)
  • All state classes use ChangeNotifier consistently (positive aspect)

Evidence:

  • lib/states/session_state.dart: Uses ChangeNotifier, stores state in both memory and database
    • Every setter saves to database immediately (lines 63-123)
    • Async init() method loads from database (lines 23-60)
  • lib/states/connectivity_status.dart: Uses ChangeNotifier pattern
  • lib/states/saving_state.dart: Uses ChangeNotifier
  • lib/states/user_loyalty_state.dart: Uses ChangeNotifier
  • lib/states/selected_gas_station_marker_state.dart: Uses ChangeNotifier
  • lib/singletons/: Multiple singletons for global state access
    • SessionStateSingleton, UserLoyaltySingleton, DatabaseSingleton, DioSingleton, FirebaseSingleton, NavigatorSingleton, ParameterStateSingleton, MapStateSingleton, ActivitySingleton, ThemeSingleton, WidgetLayoutStateSingleton
  • State initialization: sessionState.init() called separately (async pattern)
  • Direct database access: lib/database/daos/ accessed directly in some places

Risk Level: Medium Risk

Recommendation:

  • Immediate actions:
    • Review state synchronization between memory and database
    • Add state validation to prevent inconsistencies
    • Consider using flutter_secure_storage for sensitive state instead of database
  • Short-term:
    • Implement state management best practices (single source of truth)
    • Add state change notifications and observers
    • Consider using dependency injection instead of singletons for better testability
    • Separate UI state from business logic state
    • Consider batching database writes for performance

2.4 Code Duplication

Status: ⚠️Warning

Findings:

  • Firebase Performance tracing duplicated: Similar tracing code duplicated across multiple routes
  • Route setup pattern repeated: Similar setup() async method pattern in multiple routes
  • FutureBuilder pattern duplicated: Similar FutureBuilder usage across routes for loading states
  • Singleton access pattern: Similar singleton instantiation patterns across routes
  • Error handling duplicated: Similar error handling code in multiple routes
  • Dialog patterns repeated: Similar AlertDialog creation code in multiple places
  • Session state checks: Similar session state access patterns throughout routes

Evidence:

  • Firebase Performance tracing duplicated in multiple routes:
    • lib/routes/login_route.dart (lines 49-54)
    • lib/routes/generic_route.dart (lines 35-40)
    • lib/routes/register_route.dart (line 40)
    • lib/routes/register_data_route.dart (line 43)
    • lib/routes/save_gulf_route.dart (line 32)
    • lib/routes/video_route.dart (line 24)
    • And potentially more routes with identical pattern
  • Similar setup() async method pattern:
    • lib/routes/login_route.dart (lines 59-74)
    • lib/routes/generic_route.dart (lines 45-48)
    • lib/routes/register_route.dart (line 51)
    • lib/routes/register_data_route.dart (line 54)
    • lib/routes/redeem_two_route.dart (line 51)
    • lib/routes/save_gulf_route.dart (line 43)
    • Multiple routes follow same pattern
  • Similar FutureBuilder patterns for loading states across routes
  • Singleton access: SessionStateSingleton(), UserLoyaltySingleton(), etc. instantiated in multiple routes
  • Error handling: Similar .catchError() with AlertDialog patterns in multiple routes

Risk Level: Medium Risk

Recommendation:

  • Immediate actions:
    • Extract Firebase Performance tracing into a base route class or mixin
    • Create reusable widgets for common UI patterns (loading states, error states)
    • Extract common route patterns into base classes or mixins
    • Create base route class with common functionality (setup(), tracing, etc.)
  • Short-term:
    • Refactor state management classes to use inheritance or composition
    • Extract common API call patterns into service classes
    • Create reusable error handling widgets
    • Implement route factory pattern for common route structures
    • Create mixin for Firebase Performance tracing

2.5 Dead Code & Unused Files

Status: ⚠️Warning

Findings:

  • TODO comments found in codebase indicating incomplete features or future work
  • lib/widgets/layouts/models/webform_widget_layout_model.dart: Contains @todo make a form preview comment (line 40)
  • lib/widgets/layouts/models/swiper_widget_layout_model.dart: Contains TODO comments about setting correct target_activity (lines 138, 151)
  • lib/widgets/layouts/models/game_widget_layout_model.dart: Contains @todo maybe we should provide a preview here? comment (line 91)
  • Repository may contain other incomplete code that should be audited
  • No deprecated files found, but TODO comments indicate technical debt

Evidence:

  • lib/widgets/layouts/models/webform_widget_layout_model.dart: Line 40 contains // @todo make a form preview.
  • lib/widgets/layouts/models/swiper_widget_layout_model.dart: Lines 138, 151 contain // TODO: set correct target_activity in flutter_markedeer_mobile_content
  • lib/widgets/layouts/models/game_widget_layout_model.dart: Line 91 contains // @todo maybe we should provide a preview here?
  • TODO comments indicate incomplete features or future work

Risk Level: Low Risk

Recommendation:

  • Review and address TODO comments in codebase
  • Create issues or tasks for incomplete features
  • Establish process for tracking and completing TODO items
  • Consider removing or completing TODO items during refactoring
  • Use issue tracking system to manage technical debt

2.6 Best Practices & Patterns

Status: ⚠️Warning

Findings:

  • Singleton pattern used extensively, which can make testing difficult
  • Direct database access in some places bypasses state management
  • Consistent state management patterns (all use ChangeNotifier) - positive aspect
  • No clear dependency injection strategy
  • Mix of patterns (singletons, Provider, direct database access) creates complexity
  • Need strategic approach to refactoring for better testability and maintainability

Evidence:

  • Multiple singleton classes in lib/singletons/ (11+ singleton classes)
  • Direct database access in routes and models
  • Consistent state management (all state classes use ChangeNotifier)
  • No dependency injection framework in use
  • Global state access throughout codebase

Risk Level: Medium Risk

Recommendation:

  • Strategic Approach: Consider migrating from singletons to dependency injection
  • Align state management patterns (already consistent - good)
  • Create clear architecture guidelines for new code
  • Consider using a DI framework (e.g., get_it) for better testability
  • Document architectural decisions and patterns
  • Plan refactoring sprints to improve testability and maintainability
  • Separate concerns: business logic, state management, and UI layers