Refactor StoryModel: Migrate To AppState For Better Management

by Mireille Lambert 63 views

Hey guys! Today, we're diving deep into a critical refactoring task within the StoryCAD project: streamlining our application's state management by migrating StoryModel and StoryModelFile to AppState. This move is all about tidying up our architecture, squashing circular dependencies, and making our codebase more maintainable. Let's get started!

The Current State: A Web of Dependencies

Understanding the Problem

Currently, the StoryModel and StoryModelFile are accessed through OutlineViewModel, which sounds simple enough, right? But this design choice has led to a tangled web of dependencies that's making our lives harder. Multiple services are relying on OutlineViewModel solely to get to these core data pieces. Think of it like having to go through a crowded room just to grab a glass of water – inefficient and annoying!

For instance, AutoSaveService needs to know the StoryModel.StoryElements.Count and if StoryModel.Changed, and it also needs access to the SaveFile() method. BackupService, on the other hand, just wants the StoryModelFile path. And there are other services, like StoryIO and OutlineService, plus various ViewModels, that are likely hopping on this crowded route as well. It's like everyone's trying to squeeze through the same door, creating unnecessary congestion and making the system much more complex than it needs to be. This not only slows things down but also introduces a higher risk of errors and makes debugging a real headache.

The Nasty Circular Dependencies

The way things are set up now, we've got some circular dependencies rearing their ugly heads. Circular dependencies occur when two or more modules depend on each other to function correctly. This creates a loop that makes the system incredibly difficult to understand, modify, or test because changes in one area can have unexpected ripple effects elsewhere. Let’s break down the specific cycles we're dealing with:

  1. OutlineViewModel → AutoSaveService → OutlineViewModel: Our OutlineViewModel relies on the AutoSaveService to ensure that the user’s work is periodically saved, preventing data loss. However, AutoSaveService needs information from OutlineViewModel—specifically, the state and content of the StoryModel—to perform its function. This creates a direct cycle, where each component requires the other to be fully operational, leading to potential initialization and runtime issues. Imagine trying to start a car where the engine needs the battery to start, but the battery needs the engine to charge—it's a classic chicken-and-egg problem.
  2. OutlineViewModel → BackupService → OutlineViewModel: Similarly, OutlineViewModel is connected to BackupService, which is responsible for creating backups of the story files. BackupService needs to know the location of the StoryModelFile, which is managed by OutlineViewModel. Again, this forms a circular dependency, as BackupService depends on OutlineViewModel to know what to back up, and potentially OutlineViewModel could depend on BackupService for tasks related to managing backups. This cycle complicates the process of making changes or improvements to either service, as alterations in one could inadvertently affect the other.
  3. Indirect cycles through services that need story data: Beyond the direct cycles, there are indirect dependencies that add further complexity. Various other services and ViewModels across the application also need access to story data. These components might depend on OutlineViewModel to get to StoryModel and StoryModelFile, creating additional pathways that intertwine the application’s modules. This web of dependencies can make it challenging to trace the flow of data and logic, increasing the chances of introducing bugs during development and making the system more difficult to scale or maintain. It's like a crowded intersection where cars are coming from all directions, making it difficult to navigate and prone to accidents. Addressing these cycles is crucial for improving the stability and maintainability of our application.

Where's the Code Hiding?

If you're curious where all this is happening in our codebase, here's a quick rundown:

  • OutlineViewModel is the current home for StoryModel and StoryModelFile properties.
  • AutoSaveService and BackupService are accessing these properties via injected OutlineViewModel instances.
  • There's even a TODO comment in the code already acknowledging this issue – we knew this was a problem lurking in the shadows!

The Proposed Solution: Moving to AppState

Alright, so we've identified the problem. Now, let's talk about the fix! Our plan is to move the core properties – StoryModel and StoryModelFile – from OutlineViewModel to AppState. Think of AppState as the central nervous system of our application, holding the essential data that everyone needs to know. By centralizing these properties, we can break those pesky circular dependencies and simplify data access.

Step 1: AppState Gets an Upgrade

We're going to beef up our AppState class with the following:

public class AppState
{
    // Existing properties...
    
    // Move from OutlineViewModel:
    public StoryModel StoryModel { get; set; }
    public string StoryModelFile { get; set; }
    public bool StoryChanged => StoryModel?.Changed ?? false;
    public int StoryElementCount => StoryModel?.StoryElements?.Count ?? 0;
}

We're not just moving the StoryModel and StoryModelFile; we're also adding some handy properties like StoryChanged and StoryElementCount to make it even easier for services to access the information they need.

Step 2: Services Start Using AppState

Next up, we're going to update our services to use AppState instead of directly depending on OutlineViewModel. This is where the magic happens in breaking those circular dependencies!

For example, AutoSaveService will now look like this:

// AutoSaveService - instead of OutlineViewModel dependency
public AutoSaveService(AppState appState, /* other deps */)
{
    _appState = appState;
    // Access via: _appState.StoryModel, _appState.StoryChanged, etc.
}

And BackupService will follow suit:

// BackupService - instead of OutlineViewModel dependency  
public BackupService(AppState appState, /* other deps */)
{
    _appState = appState;
    // Access via: _appState.StoryModelFile
}

Now, these services can grab the data they need directly from AppState, without going through OutlineViewModel.

Step 3: OutlineViewModel Becomes an AppState Proxy

To keep things smooth during the transition, we're going to have OutlineViewModel act as a proxy for AppState. This means it will still expose the StoryModel and StoryModelFile properties, but under the hood, it will be pulling them from AppState.

public class OutlineViewModel
{
    private readonly AppState _appState;
    
    public StoryModel StoryModel => _appState.StoryModel;
    public string StoryModelFile => _appState.StoryModelFile;
    // Maintain existing API for compatibility during transition
}

This way, we can update the services one by one without breaking everything in the process. It’s like using a temporary bridge to cross a river while the main bridge is under construction. We maintain the flow of traffic (or, in our case, data) while making necessary improvements.

Step 4: The Tricky SaveFile() Method

The SaveFile() method in OutlineViewModel is a bit of a special case. We need to figure out the best way to handle this. We've got a few options on the table:

  • Option A: Move it to a dedicated StoryPersistenceService. This would keep our persistence logic nicely encapsulated in its own service, making it easier to test and maintain. It's like giving the task of safeguarding the story's content to a specialized librarian, ensuring that it's well-preserved and easily accessible.
  • Option B: Use messaging (SaveRequestedMessage). This approach would allow us to decouple the save operation from the UI, making the system more flexible and responsive. Think of it as sending a request through the postal service; the sender doesn't need to know exactly how the message will be delivered, just that it will reach its destination. This method is particularly useful in complex applications where different components need to communicate without direct dependencies.
  • Option C: Keep it in OutlineViewModel but accessed via an interface. This is a more conservative approach that would allow us to defer the decision of where to ultimately put the save logic. It's like setting up a temporary office in a familiar building; you're still in a known environment, but you're preparing for a potential move to a new, more specialized location.

Acceptance Criteria: How We Know We've Succeeded

To make sure we're on the right track, we've got a clear set of acceptance criteria. These are the goals we need to hit to consider this migration a success:

  • [ ] StoryModel property exists in AppState
  • [ ] StoryModelFile property exists in AppState
  • [ ] AutoSaveService no longer depends on OutlineViewModel
  • [ ] BackupService no longer depends on OutlineViewModel
  • [ ] All services that need story data use AppState instead of OutlineViewModel
  • [ ] OutlineViewModel uses AppState for story data (no duplication)
  • [ ] Circular dependencies OutlineViewModel ↔ AutoSaveService eliminated
  • [ ] Circular dependencies OutlineViewModel ↔ BackupService eliminated
  • [ ] All existing tests pass
  • [ ] Application smoke test passes (load, edit, save story)

Implementation Steps: The Road Map

Here's a step-by-step guide to how we're going to tackle this migration:

  1. Add StoryModel and StoryModelFile properties to AppState
  2. Update AppState initialization to set these properties
  3. Change AutoSaveService to depend on AppState instead of OutlineViewModel
  4. Change BackupService to depend on AppState instead of OutlineViewModel
  5. Update OutlineViewModel to proxy to AppState for these properties
  6. Find and update all other services using OutlineViewModel for story data
  7. Handle the SaveFile() method (separate decision needed)
  8. Update unit tests
  9. Full regression test

Testing Requirements: Ensuring Quality

Testing is crucial to ensure that our changes haven't broken anything. Here's what we'll be doing to verify our work:

  • Unit tests for AppState with new properties
  • Unit tests for AutoSaveService with AppState dependency
  • Unit tests for BackupService with AppState dependency
  • Integration test for save/load flow
  • Integration test for auto-save functionality
  • Integration test for backup functionality
  • Manual verification of:
    • Creating new story
    • Loading existing story
    • Auto-save triggers
    • Manual save
    • Backup creation

Impact Analysis: What's at Stake?

This is a High Impact change because it touches so many parts of our application. We're talking about:

  • All services that access story data
  • Save/load functionality
  • Auto-save system
  • Backup system
  • Potentially all ViewModels that display story data

Risk Mitigation: Playing it Safe

To minimize the risk, we're going to:

  • Keep OutlineViewModel properties as proxies initially
  • Migrate services incrementally if needed
  • Do extensive testing at each step

Related Issues: Connecting the Dots

This task is related to a few other issues in our project:

  • Parent: #1063 (Change IOC Resolution)
  • Related: Circular Dependency Elimination Plan
  • Blocks: Multiple service → ViewModel dependency fixes

Notes: Final Thoughts

This is a long-standing architectural issue, and tackling it will significantly simplify our dependency graph. It's a crucial step before we dive into other circular dependency fixes. We might even consider moving other OutlineViewModel properties to AppState down the road. This is all about making our application more robust, maintainable, and a joy to work with. Let’s get it done!