ADR: Refactor Core Class Static State Management

Status

Implemented (Date: 2025-06-26)

Context

Our current Core class implementation uses static fields to store application state, which has created significant challenges for testing and maintainability over the past three years. The development team has experienced a 3:1 ratio of test code to feature code, with much of this overhead attributed to difficulties working with the static state architecture.

Current Problems

  • Test Isolation Issues: Tests interfere with each other due to shared static state

  • Difficult Test Setup: Creating isolated test scenarios requires complex workarounds

  • Mocking Limitations: Static dependencies make it nearly impossible to inject test doubles

  • Maintenance Overhead: High ratio of test maintenance code to actual feature development

Technical Debt Impact

The static state pattern has resulted in:

  • Brittle test suites where test order affects outcomes

  • Increased development time for writing reliable tests

  • Reduced confidence in test results due to intermittent failures

  • Difficulty in implementing proper unit testing practices

Decision

We will refactor the Core class to use dependency injection and extract state management into a separate CoreState class.

Proposed Architecture

Current: Core (static fields + logic)
Proposed: Core (logic) → CoreState (state management)

Implementation Strategy

  1. Create CoreState Class: Extract all static state into a dedicated state management class

  2. Implement Singleton Pattern: Ensure single instance behavior is maintained for production use

  3. Add Dependency Injection: Allow Core to accept CoreState instance via constructor or setter

  4. Maintain API Compatibility: Preserve existing public interface to avoid breaking changes

  5. Gradual Migration: Implement changes incrementally to minimize risk

Consequences

Positive (Achieved)

  • Improved Testability: TestCoreState enables isolated testing with custom state configurations

  • Better Thread Safety: Synchronized blocks and proper SaveThread lifecycle management prevent race conditions

  • Enhanced Maintainability: Clear separation between state management and business logic

  • Test Infrastructure: Dedicated testing utilities reduce test complexity and improve reliability

  • Future Flexibility: CoreState architecture enables easier feature development and testing

Additional Benefits Delivered

  • Auto-save Reliability: Improved thread lifecycle management reduces potential memory leaks

  • Code Quality: Cleaner deprecation handling with explicit removal indicators

  • Developer Experience: Easier test writing with customizable state handling

Implementation Considerations

  • Learning Curve: Team successfully adapted to new CoreState pattern

  • Code Review: Comprehensive review completed through PR process

  • Backward Compatibility: Maintained while improving internal architecture

Implementation Results

The refactoring has been successfully implemented with the following changes:

Core Refactoring Delivered

  • CoreState Class: Successfully extracted state management (IProject, IMainWindow, IEditor) from Core class

  • State Centralization: All static state now managed through centralized CoreState instance

  • API Compatibility: Maintained backward compatibility while improving internal architecture

Additional Improvements Implemented

  • Auto-save Enhancement: Improved SaveThread with proper lifecycle management through fin() method

  • Thread Safety: Added synchronized blocks and better thread management to prevent race conditions

  • Test Infrastructure: Created TestCoreState class for customizable state handling in unit tests

  • Deprecation Cleanup: Updated deprecated methods with forRemoval = true for clearer migration path

Testing Benefits Realized

  • Test Isolation: Unit tests can now use TestCoreState for isolated testing scenarios

  • Improved Reliability: Better thread lifecycle management reduces test flakiness

  • Maintainability: Cleaner separation of concerns between state and business logic

Alternatives Considered

Alternative 1: Keep Current Architecture

  • Pros: No development effort required

  • Cons: Continues current testing problems, technical debt accumulates

Alternative 2: Complete Rewrite

  • Pros: Clean slate approach

  • Cons: High risk, significant development time, potential for introducing bugs

Alternative 3: Partial Static Cleanup

  • Pros: Lower effort than full refactor

  • Cons: Only addresses some issues, doesn’t solve core testing problems

Decision Makers

  • Proposed by: Hiroshi Miura

  • Review Required: Development Team

Success Metrics - Results

Testing Improvements

  • Test Isolation: ✅ Achieved through TestCoreState implementation

  • Test Reliability: ✅ Improved with better thread safety and lifecycle management

  • Test Infrastructure: ✅ Dedicated testing utilities implemented

  • Maintainability: ✅ Cleaner state management architecture delivered

Technical Improvements

  • Thread Safety: ✅ Enhanced with synchronized blocks and proper SaveThread management

  • Code Quality: ✅ Improved with cleaner deprecation handling

  • Architecture: ✅ Successfully separated state management from business logic

Next Steps

  • Monitor test suite performance and reliability improvements

  • Gather developer feedback on new testing patterns

  • Document best practices for using TestCoreState in future tests

  • Consider extending CoreState pattern to other components if beneficial

References

  • Relevant Mail Thread: SourceForge Mailing List Discussion

  • “Head First Design Patterns” - Singleton and Dependency Injection patterns

  • “Working Effectively with Legacy Code” by Michael Feathers - Techniques for making legacy code testable

  • Team testing pain points documented over 3-year period

This ADR documents the completed implementation of Core class refactoring. The solution successfully addresses the original testing and maintainability concerns while providing additional improvements in thread safety and code quality.