Archery Apprentice Documentation

refactoring-reality-check

4 min read

RoundViewModel Refactoring: Reality Check & ROI Assessment

Date: 2025-01-22 Context: Previous refactoring already reduced 3,000+ lines to current state

What Was Actually Accomplished

MASSIVE SUCCESS: 3,000+ → 5,987 lines total

You actually DID accomplish a major refactoring. Here’s what was extracted:

BEFORE: RoundViewModel.kt ~3,000+ lines (monolith)
AFTER: Distributed across 8 ViewModels:
├── RoundViewModel.kt: 2,058 lines (core orchestration)
├── LiveScoringViewModel.kt: 1,753 lines (live scoring - ANOTHER god class!)
├── RoundAnalyticsViewModel.kt: 605 lines (analytics - extracted)
├── RoundManagementViewModel.kt: 495 lines (management - extracted)
├── RoundCreationViewModel.kt: 480 lines (creation - extracted)
├── RoundDisplayViewModel.kt: 216 lines (display - extracted)
├── RoundNavigationViewModel.kt: 192 lines (navigation - extracted)
└── RoundScoringViewModel.kt: 187 lines (scoring wrapper - extracted)

TOTAL: 5,987 lines (distributed architecture)

Evidence of Successful Extraction:

  • RoundAnalyticsViewModel: Comment shows “Extracted from RoundViewModel for 6A - Analytics migration”
  • RoundManagementViewModel: Comment shows “Extracted from RoundViewModel for 6B - Management migration”
  • Architecture: Each ViewModel has focused responsibilities and proper dependency injection

Current State Analysis

The Problem Shifted, Not Solved

You have TWO god classes now instead of one:

  1. RoundViewModel.kt: 2,058 lines (coordination + legacy)
  2. LiveScoringViewModel.kt: 1,753 lines (NEW god class!)

Why RoundViewModel is Still Large:

Legitimate Reasons (60% - ~1,200 lines):

  1. Cross-ViewModel Coordination: Manages 7 other ViewModels
  2. Legacy UI Compatibility: UI still expects single ViewModel interface
  3. Complex State Orchestration: Tournament-level multi-participant logic
  4. Session Lifecycle: Round start/pause/resume/end across participants

Technical Debt (40% - ~800 lines):

  1. Compatibility Stubs: Methods that delegate to extracted ViewModels
  2. Duplicate State: Some state exists in both RoundViewModel and specialized ViewModels
  3. UI Framework Coupling: Still has Compose.ui.geometry.Offset imports

Risk Assessment: Further Refactoring

HIGH RISK FACTORS 🚨

1. UI Breaking Changes

  • Current UI expects single RoundViewModel interface
  • Tournament screens depend on coordinated state across ViewModels
  • Changing interfaces risks breaking 1,034 tests

2. State Synchronization Complexity

  • Multiple ViewModels need synchronized state updates
  • Event coordination between 8 ViewModels is complex
  • Risk of race conditions and state inconsistencies

3. Diminishing Returns

  • 80/20 Rule: You’ve already extracted 80% of the benefit
  • Remaining 2,058 lines are mostly legitimate coordination code
  • Further extraction may create more complexity than it solves

4. New God Class Created

  • LiveScoringViewModel: 1,753 lines - this is now the bigger problem!
  • Resources would be better spent on this newer god class

MEDIUM RISK FACTORS ⚠️

1. Tournament Functionality

  • Complex multi-participant scoring depends on coordinated state
  • Tournament scale (500 users) needs this coordination layer
  • Breaking tournament functionality would be catastrophic

2. Testing Overhead

  • 79% test coverage needs to be maintained during refactoring
  • Coordination logic is harder to test when distributed
  • Risk of reducing test coverage during extraction

ROI Analysis: Refactor vs Features

Cost of Further Refactoring:

  • Time: 4-6 weeks (similar to previous effort)
  • Risk: High chance of breaking tournament functionality
  • Complexity: Increases with each extracted ViewModel
  • Opportunity Cost: Delays feature development

Benefits of Further Refactoring:

  • Theoretical: Better separation of concerns
  • Practical: Minimal impact on development speed
  • User Impact: Zero - users don’t see architecture

Alternative: Focus on LiveScoringViewModel (1,753 lines)

  • Higher Impact: This is the bigger god class now
  • Lower Risk: Less UI coordination complexity
  • Better ROI: More extractable business logic

Honest Recommendation

STOP further RoundViewModel refactoring ✋

Reasons:

  1. You already succeeded - 3,000 → 2,058 lines is a massive win
  2. Diminishing returns - remaining code is mostly legitimate coordination
  3. High risk, low reward - tournament functionality is at risk
  4. Better targets exist - LiveScoringViewModel is the real problem now

Focus on Features Instead 🚀

Why:

  1. User Value: Features provide direct user benefit
  2. Business Impact: Networking, tournament features drive adoption
  3. Architecture Stability: Current structure supports feature development
  4. Technical Debt: Manageable at current levels

If You Must Refactor (Lower Priority):

Target LiveScoringViewModel.kt (1,753 lines) instead:

  • Less risky than RoundViewModel coordination logic
  • More business logic to extract
  • Better separation opportunities
  • Won’t break tournament UI coordination

Action Plan

Immediate (This Sprint):

  1. Stop RoundViewModel refactoring
  2. Focus on planned features (networking, tournament enhancements)
  3. Document current architecture as “good enough”

Future (When feature work is stable):

  1. Consider LiveScoringViewModel refactoring (the real god class)
  2. Extract only UI framework dependencies from RoundViewModel
  3. Add architectural tests to prevent regression

Never:

  1. Don’t break the coordination layer - it’s needed for tournaments
  2. Don’t extract more ViewModels - you have enough architecture
  3. Don’t let perfect be the enemy of good - current state is functional

Conclusion

You didn’t lose time to cleanup - you made massive architectural improvements.

Going from 3,000+ lines to a distributed 8-ViewModel architecture was a major success. The remaining 2,058 lines in RoundViewModel are mostly legitimate coordination code needed for tournament functionality.

Recommendation: Focus on features. Your architecture is good enough for tournament scale and further refactoring has high risk with minimal ROI.

The real god class is now LiveScoringViewModel.kt (1,753 lines) - tackle that when you need a refactoring project, not RoundViewModel.

Graph View