Skip to content

Refactor: Replace Abstract Factory with Single Analysis Class #9

Description

@cnicholas

Summary

Refactor the analysis system to replace the Abstract Factory pattern with a Single Analysis Class using the Strategy Pattern. This will significantly improve code maintainability and reduce complexity.

Current Problems

  1. Over-engineered: Abstract Factory pattern adds ~60 lines of boilerplate for 4 simple classes
  2. Scattered logic: Analysis logic spread across 4 separate classes (Xbar, Sbar, IMR, R)
  3. Code duplication: Similar patterns repeated in each analysis class
  4. Hard to maintain: Changes require updates across multiple classes and factory methods
  5. Unnecessary abstractions: AbstractFactory, AbstractAnalysis, and AbstractAnalysisSpecification add complexity without clear benefit

Proposed Solution: Single Analysis Class with Strategy Pattern

Architecture

Replace the factory and 4 analysis classes with one Analysis class that uses internal strategy methods:

class Analysis:
    """Single analysis class handling all chart types via strategy methods"""
    
    def __init__(self, df: pd.DataFrame, specification: dict):
        self.raw_df = df
        self.spec = AnalysisSpecification(specification['analysis_type'], specification)
        self.ads = AnalysisDataSet(df, self.spec)
        self.analysis_type = specification['analysis_type']
        
    def calculate(self) -> pd.DataFrame:
        """Dispatch to appropriate calculation method"""
        strategies = {
            'Xbar': self._calculate_xbar,
            'S': self._calculate_s,
            'Imr': self._calculate_imr,
            'R': self._calculate_r
        }
        
        if self.analysis_type not in strategies:
            raise ValueError(f'Analysis type {self.analysis_type} not supported!')
            
        return strategies[self.analysis_type]()
    
    def _calculate_xbar(self) -> pd.DataFrame:
        """Calculate Xbar chart statistics"""
        # Move logic from Xbar.calculate_statistics()
        pass
    
    def _calculate_s(self) -> pd.DataFrame:
        """Calculate S chart statistics"""
        # Move logic from Sbar
        pass
    
    def _calculate_imr(self) -> pd.DataFrame:
        """Calculate IMR chart statistics"""
        # Move logic from IMR
        pass
    
    def _calculate_r(self) -> pd.DataFrame:
        """Calculate R chart statistics"""
        # Move logic from R
        pass

Simplified Entry Point

def perform_analysis(df: pd.DataFrame, specification: dict) -> pd.DataFrame:
    """Simplified analysis entry point"""
    return Analysis(df, specification).calculate()

Benefits

1. Massive Code Reduction

  • Removes ~60 lines of factory boilerplate
  • Reduces file size from ~1750 to ~1100 lines (37% reduction)
  • Eliminates 3 abstract base classes

2. Improved Maintainability

  • All analysis logic in one place
  • Easier to understand the flow
  • Shared helper methods naturally accessible
  • Single point of entry for all analyses

3. Better Testing

  • Test one class with different analysis types
  • Mock/patch strategies individually
  • Simpler test fixtures

4. More Pythonic

  • Python favors composition over inheritance
  • Strategy pattern is clearer than factory pattern
  • Follows "flat is better than nested" principle

5. Easier Extension

  • Adding new analysis type = adding one method
  • No need to touch factory or abstract classes
  • Clear pattern to follow

Implementation Plan

Phase 1: Preparation (No Breaking Changes)

  1. ✅ Create this issue
  2. ✅ Create refactor-analysis-class branch
  3. Create new Analysis class alongside existing code
  4. Implement strategy methods by moving logic from existing classes
  5. Ensure all tests pass with new implementation

Phase 2: Migration

  1. Update perform_analysis() to use new Analysis class
  2. Keep old classes temporarily for comparison
  3. Run full test suite to verify equivalence

Phase 3: Cleanup

  1. Remove old factory classes (AbstractFactory, AnalysisFactory)
  2. Remove old analysis classes (Xbar, Sbar, IMR, R)
  3. Remove AbstractAnalysis base class
  4. Update any remaining references
  5. Final test run

Phase 4: Polish

  1. Add docstrings to strategy methods
  2. Update any documentation
  3. Create PR with before/after comparison

Code to Remove

  • AbstractFactory class (~15 lines)
  • AnalysisFactory class (~15 lines)
  • AbstractAnalysis class (~10 lines)
  • Xbar class (~155 lines)
  • Sbar class (~15 lines)
  • IMR class (~20 lines)
  • R class (~20 lines)

Total removal: ~250 lines

Code to Add

  • Analysis class with strategy methods (~200 lines including moved logic)

Net reduction: ~50 lines + elimination of complex inheritance

Testing Requirements

  • All existing tests must pass
  • No changes to test files required (same inputs/outputs)
  • Verify equivalence with old implementation before removal

Success Criteria

  • ✅ All 27 tests pass
  • ✅ Zero warnings
  • ✅ Code reduction of at least 30 lines
  • ✅ Factory pattern completely removed
  • ✅ Single entry point for all analyses
  • ✅ Easier to understand for new contributors

Related Issues

  • Addresses over-engineering concerns
  • Improves maintainability for future features
  • Makes codebase more accessible

Labels

enhancement, refactoring, tech-debt

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions