plan for refactoring

2025-08-17 10:07:34 +02:00
parent 70e4b4e89f
commit 2e4a981254
2 changed files with 706 additions and 0 deletions
--- a/REFACTORING_ANALYSIS.md
+++ b/REFACTORING_ANALYSIS.md
@ -0,0 +1,429 @@
+# Tetris C++ SDL3 - Code Refactoring Analysis
+
+## Executive Summary
+
+The `main.cpp` file currently serves as both the application entry point and contains a significant amount of game logic, rendering code, and UI management. This violates several SOLID principles and makes the codebase difficult to maintain, test, and extend. This analysis provides a comprehensive refactoring plan to improve code organization, maintainability, and adherence to best practices.
+
+## Current Issues Analysis
+
+### 1. **Single Responsibility Principle (SRP) Violations**
+
+**Current State:** The `main()` function handles:
+- SDL initialization and cleanup
+- Asset loading (textures, fonts, sounds)
+- Game loop management
+- Event handling for multiple states
+- Rendering logic for all game states
+- State management
+- Input handling (keyboard, mouse)
+- Audio management
+- Background effects management
+
+**Impact:** 
+- Function is ~1,678 lines long
+- Difficult to test individual components
+- High coupling between unrelated functionality
+- Hard to debug specific issues
+
+### 2. **Open/Closed Principle (OCP) Violations**
+
+**Current Issues:**
+- Adding new game states requires modifying the main loop
+- New input handling requires changing the main event handling code
+- New rendering features require modifying the massive switch statement
+- Hard-coded state transitions throughout the main function
+
+### 3. **Dependency Inversion Principle (DIP) Violations**
+
+**Current Issues:**
+- Direct instantiation of concrete classes in main
+- Tight coupling to SDL-specific implementations
+- No abstraction layer for rendering, audio, or input systems
+- Global state variables scattered throughout
+
+### 4. **Code Organization Issues**
+
+**Current Problems:**
+- Mixing of high-level application logic with low-level SDL code
+- Inline lambda functions making code hard to read
+- Global static variables and functions
+- No clear separation between initialization, game loop, and cleanup
+- Duplicated code in event handling
+- Magic numbers and constants scattered throughout
+
+## Proposed Refactoring Strategy
+
+### Phase 1: Extract Core Managers
+
+#### 1.1 Application Manager
+Create an `ApplicationManager` class to handle the application lifecycle:
+
+```cpp
+class ApplicationManager {
+public:
+    bool initialize();
+    void run();
+    void shutdown();
+    
+private:
+    std::unique_ptr<RenderManager> m_renderManager;
+    std::unique_ptr<InputManager> m_inputManager;
+    std::unique_ptr<StateManager> m_stateManager;
+    std::unique_ptr<AssetManager> m_assetManager;
+    std::unique_ptr<AudioManager> m_audioManager;
+};
+```
+
+#### 1.2 Render Manager
+Extract all rendering logic:
+
+```cpp
+class RenderManager {
+public:
+    bool initialize(int width, int height);
+    void beginFrame();
+    void endFrame();
+    void setViewport(const Viewport& viewport);
+    void renderTexture(SDL_Texture* texture, const Rect& src, const Rect& dst);
+    void renderRect(const Rect& rect, const Color& color);
+    
+private:
+    SDL_Window* m_window;
+    SDL_Renderer* m_renderer;
+    Viewport m_currentViewport;
+};
+```
+
+#### 1.3 Input Manager
+Centralize input handling:
+
+```cpp
+class InputManager {
+public:
+    void processEvents();
+    bool isKeyPressed(SDL_Scancode key) const;
+    bool isKeyHeld(SDL_Scancode key) const;
+    Point getMousePosition() const;
+    bool isMouseButtonPressed(int button) const;
+    
+    void registerKeyHandler(SDL_Scancode key, std::function<void()> handler);
+    void registerMouseHandler(int button, std::function<void(Point)> handler);
+    
+private:
+    std::unordered_map<SDL_Scancode, bool> m_keyStates;
+    std::unordered_map<SDL_Scancode, bool> m_previousKeyStates;
+    Point m_mousePosition;
+    std::unordered_map<int, bool> m_mouseStates;
+};
+```
+
+#### 1.4 Asset Manager
+Manage all game assets:
+
+```cpp
+class AssetManager {
+public:
+    bool loadTexture(const std::string& name, const std::string& path);
+    bool loadFont(const std::string& name, const std::string& path, int size);
+    bool loadSound(const std::string& name, const std::string& path);
+    
+    SDL_Texture* getTexture(const std::string& name) const;
+    FontAtlas* getFont(const std::string& name) const;
+    // etc.
+    
+private:
+    std::unordered_map<std::string, std::unique_ptr<SDL_Texture, SDLDeleter>> m_textures;
+    std::unordered_map<std::string, std::unique_ptr<FontAtlas>> m_fonts;
+    // etc.
+};
+```
+
+### Phase 2: Improve State Management
+
+#### 2.1 Enhanced State Pattern
+Improve the current state system:
+
+```cpp
+class IGameState {
+public:
+    virtual ~IGameState() = default;
+    virtual void onEnter() = 0;
+    virtual void onExit() = 0;
+    virtual void update(float deltaTime) = 0;
+    virtual void render(RenderManager& renderer) = 0;
+    virtual void handleEvent(const SDL_Event& event) = 0;
+    virtual AppState getType() const = 0;
+};
+
+class StateManager {
+public:
+    void pushState(std::unique_ptr<IGameState> state);
+    void popState();
+    void changeState(std::unique_ptr<IGameState> state);
+    void update(float deltaTime);
+    void render(RenderManager& renderer);
+    void handleEvent(const SDL_Event& event);
+    
+private:
+    std::stack<std::unique_ptr<IGameState>> m_states;
+};
+```
+
+#### 2.2 State Factory
+Create states using a factory pattern:
+
+```cpp
+class StateFactory {
+public:
+    static std::unique_ptr<IGameState> createState(
+        AppState type, 
+        const StateContext& context
+    );
+    
+private:
+    static std::unordered_map<AppState, std::function<std::unique_ptr<IGameState>(const StateContext&)>> m_creators;
+};
+```
+
+### Phase 3: Separate Rendering Systems
+
+#### 3.1 UI Renderer
+Extract UI rendering to separate class:
+
+```cpp
+class UIRenderer {
+public:
+    void renderButton(const Button& button);
+    void renderPopup(const Popup& popup);
+    void renderProgressBar(const ProgressBar& progressBar);
+    void renderMenu(const Menu& menu);
+    
+private:
+    RenderManager& m_renderManager;
+    AssetManager& m_assetManager;
+};
+```
+
+#### 3.2 Game Renderer
+Separate game-specific rendering:
+
+```cpp
+class GameRenderer {
+public:
+    void renderGameBoard(const Game& game, const GameLayout& layout);
+    void renderGamePiece(const Game::Piece& piece, const Transform& transform);
+    void renderBackground(BackgroundType type, int level);
+    void renderHUD(const GameStats& stats, const HUDLayout& layout);
+    
+private:
+    RenderManager& m_renderManager;
+    AssetManager& m_assetManager;
+};
+```
+
+### Phase 4: Configuration Management
+
+#### 4.1 Configuration System
+Create a centralized configuration system:
+
+```cpp
+class ConfigManager {
+public:
+    static ConfigManager& getInstance();
+    
+    template<typename T>
+    T getValue(const std::string& key, const T& defaultValue = T{}) const;
+    
+    template<typename T>
+    void setValue(const std::string& key, const T& value);
+    
+    bool loadFromFile(const std::string& filename);
+    bool saveToFile(const std::string& filename);
+    
+private:
+    std::unordered_map<std::string, std::any> m_values;
+};
+```
+
+#### 4.2 Constants Organization
+Move magic numbers to configuration:
+
+```cpp
+namespace Config {
+    namespace Window {
+        constexpr int DEFAULT_WIDTH = 1200;
+        constexpr int DEFAULT_HEIGHT = 1000;
+    }
+    
+    namespace Gameplay {
+        constexpr double DAS_DELAY = 170.0;
+        constexpr double ARR_RATE = 40.0;
+        constexpr float LEVEL_FADE_DURATION = 3500.0f;
+    }
+    
+    namespace UI {
+        constexpr float MIN_MARGIN = 40.0f;
+        constexpr float PANEL_WIDTH = 180.0f;
+        constexpr float PANEL_SPACING = 30.0f;
+    }
+}
+```
+
+### Phase 5: Event System
+
+#### 5.1 Event Bus
+Implement a decoupled event system:
+
+```cpp
+template<typename EventType>
+class EventBus {
+public:
+    using Handler = std::function<void(const EventType&)>;
+    using HandlerId = uint32_t;
+    
+    HandlerId subscribe(Handler handler);
+    void unsubscribe(HandlerId id);
+    void publish(const EventType& event);
+    
+private:
+    std::unordered_map<HandlerId, Handler> m_handlers;
+    HandlerId m_nextId = 1;
+};
+```
+
+#### 5.2 Game Events
+Define specific game events:
+
+```cpp
+struct GameEvents {
+    struct LevelUp { int newLevel; };
+    struct LineCleared { int linesCleared; std::vector<int> clearedRows; };
+    struct GameOver { int finalScore; int totalLines; int finalLevel; };
+    struct PieceSpawned { PieceType type; };
+    struct PieceLocked { Game::Piece piece; };
+};
+```
+
+### Phase 6: Dependency Injection
+
+#### 6.1 Service Container
+Implement a simple service container:
+
+```cpp
+class ServiceContainer {
+public:
+    template<typename T>
+    void registerService(std::shared_ptr<T> service);
+    
+    template<typename T>
+    std::shared_ptr<T> getService() const;
+    
+    template<typename T>
+    bool hasService() const;
+    
+private:
+    std::unordered_map<std::type_index, std::shared_ptr<void>> m_services;
+};
+```
+
+## Implementation Priority
+
+### High Priority (Critical Issues)
+1. **Extract Application Manager** - Reduce main() function complexity
+2. **Separate Render Manager** - Abstract SDL rendering code
+3. **Improve State Management** - Make states truly independent
+4. **Extract Input Manager** - Centralize input handling
+
+### Medium Priority (Maintainability)
+5. **Configuration System** - Remove magic numbers
+6. **Asset Manager** - Centralize resource management
+7. **UI Renderer** - Separate UI from game rendering
+8. **Event System** - Decouple components
+
+### Low Priority (Quality of Life)
+9. **Service Container** - Improve dependency management
+10. **Logging System** - Better debugging support
+11. **Performance Profiler** - Identify bottlenecks
+12. **Unit Testing Framework** - Ensure code quality
+
+## Benefits of Refactoring
+
+### Code Quality
+- **Reduced Complexity**: Main function becomes ~50 lines instead of 1,678
+- **Better Testability**: Each component can be unit tested in isolation
+- **Improved Maintainability**: Changes to one system don't affect others
+- **Enhanced Readability**: Clear separation of concerns
+
+### Development Velocity
+- **Easier Feature Addition**: New states/features don't require main() changes
+- **Parallel Development**: Multiple developers can work on different systems
+- **Faster Debugging**: Issues isolated to specific components
+- **Simplified Integration**: Clean interfaces between systems
+
+### System Reliability
+- **Reduced Coupling**: Changes have limited blast radius
+- **Better Error Handling**: Each system can handle its own errors
+- **Resource Management**: Clear ownership and lifecycle management
+- **Memory Safety**: RAII patterns throughout
+
+## Migration Strategy
+
+### Phase 1: Foundation (Week 1-2)
+1. Create basic manager interfaces
+2. Extract ApplicationManager with minimal functionality
+3. Move SDL initialization/cleanup to RenderManager
+4. Basic integration testing
+
+### Phase 2: Core Systems (Week 3-4)
+1. Implement InputManager
+2. Enhance StateManager
+3. Create AssetManager
+4. Update existing states to use new managers
+
+### Phase 3: Rendering (Week 5-6)
+1. Extract UI rendering logic
+2. Separate game rendering
+3. Implement background management
+4. Optimize rendering pipeline
+
+### Phase 4: Configuration (Week 7)
+1. Implement configuration system
+2. Move constants to config files
+3. Add runtime configuration updates
+
+### Phase 5: Events (Week 8)
+1. Implement event bus
+2. Convert callbacks to events
+3. Decouple audio system
+
+### Phase 6: Polish (Week 9-10)
+1. Add service container
+2. Implement comprehensive logging
+3. Add performance monitoring
+4. Complete documentation
+
+## Testing Strategy
+
+### Unit Tests
+- Test each manager class in isolation
+- Mock dependencies using interfaces
+- Test edge cases and error conditions
+- Achieve >90% code coverage
+
+### Integration Tests
+- Test manager interactions
+- Verify state transitions
+- Test complete game scenarios
+- Performance regression testing
+
+### System Tests
+- Full application testing
+- User interaction scenarios
+- Cross-platform compatibility
+- Memory leak detection
+
+## Conclusion
+
+This refactoring plan addresses the major architectural issues in the current codebase while maintaining compatibility with existing functionality. The modular approach allows for incremental implementation without breaking the existing game. The end result will be a maintainable, testable, and extensible codebase that follows modern C++ best practices and SOLID principles.
+
+The investment in refactoring will pay dividends in reduced development time, fewer bugs, and easier feature implementation in the future.
--- a/REFACTORING_TODO.md
+++ b/REFACTORING_TODO.md
@ -0,0 +1,277 @@
+# Tetris Refactoring TODO List
+
+## 🚀 Phase 1: Foundation (Week 1-2) - CRITICAL
+
+### Core Architecture Setup
+- [ ] Create `ApplicationManager` class
+  - [ ] Design interface and basic structure
+  - [ ] Implement initialize(), run(), shutdown() methods
+  - [ ] Move SDL initialization from main() to ApplicationManager
+  - [ ] Add proper error handling and cleanup
+  - [ ] Test basic application lifecycle
+
+- [ ] Extract `RenderManager` class
+  - [ ] Create rendering abstraction layer
+  - [ ] Move SDL_Window and SDL_Renderer management
+  - [ ] Implement viewport and scaling logic
+  - [ ] Add texture and primitive rendering methods
+  - [ ] Test rendering pipeline isolation
+
+- [ ] Implement basic `StateManager` improvements
+  - [ ] Enhance current StateManager with stack support
+  - [ ] Add state validation and error handling
+  - [ ] Implement proper state lifecycle management
+  - [ ] Test state transitions thoroughly
+
+### Immediate Code Cleanup
+- [ ] Remove global variables from main.cpp
+  - [ ] Move static variables to appropriate managers
+  - [ ] Remove global texture and font variables
+  - [ ] Eliminate global state flags
+  - [ ] Clean up static function declarations
+
+- [ ] Extract constants to configuration
+  - [ ] Create Config namespace with all constants
+  - [ ] Remove magic numbers from main.cpp
+  - [ ] Define window size constants
+  - [ ] Set up gameplay timing constants
+
+## 🔧 Phase 2: Core Systems (Week 3-4) - HIGH PRIORITY
+
+### Input Management
+- [ ] Create `InputManager` class
+  - [ ] Implement keyboard state tracking
+  - [ ] Add mouse input handling
+  - [ ] Create event handler registration system
+  - [ ] Implement DAS/ARR logic in InputManager
+  - [ ] Test input responsiveness and accuracy
+
+- [ ] Refactor event handling in main()
+  - [ ] Move SDL event polling to InputManager
+  - [ ] Replace inline event handlers with registered callbacks
+  - [ ] Simplify main loop event processing
+  - [ ] Test all input scenarios (keyboard, mouse, gamepad)
+
+### Asset Management
+- [ ] Create `AssetManager` class
+  - [ ] Design resource loading interface
+  - [ ] Implement texture management
+  - [ ] Add font loading and management
+  - [ ] Create sound asset handling
+  - [ ] Add resource cleanup and error handling
+
+- [ ] Migrate existing asset loading
+  - [ ] Move texture loading from main() to AssetManager
+  - [ ] Convert font initialization to use AssetManager
+  - [ ] Update StateContext to use AssetManager
+  - [ ] Test asset loading and memory management
+
+### State System Enhancement
+- [ ] Implement `IGameState` interface
+  - [ ] Define clear state contract
+  - [ ] Add update(), render(), handleEvent() methods
+  - [ ] Implement proper state lifecycle
+  - [ ] Add state type identification
+
+- [ ] Create `StateFactory` pattern
+  - [ ] Design state creation interface
+  - [ ] Implement factory registration system
+  - [ ] Add state parameter passing
+  - [ ] Test dynamic state creation
+
+## 🎨 Phase 3: Rendering Systems (Week 5-6) - MEDIUM PRIORITY
+
+### UI Rendering Separation
+- [ ] Create `UIRenderer` class
+  - [ ] Extract button rendering logic
+  - [ ] Implement popup rendering system
+  - [ ] Add progress bar rendering
+  - [ ] Create menu rendering components
+  - [ ] Test UI rendering consistency
+
+- [ ] Refactor state rendering
+  - [ ] Update LoadingState to use UIRenderer
+  - [ ] Modify MenuState rendering
+  - [ ] Enhance LevelSelectorState visuals
+  - [ ] Test all state rendering paths
+
+### Game Rendering Optimization
+- [ ] Create `GameRenderer` class
+  - [ ] Extract game board rendering
+  - [ ] Implement piece rendering system
+  - [ ] Add background management
+  - [ ] Create HUD rendering components
+  - [ ] Optimize rendering performance
+
+- [ ] Enhance visual effects
+  - [ ] Improve line clearing effects
+  - [ ] Add piece drop animations
+  - [ ] Enhance particle systems
+  - [ ] Implement screen transitions
+
+### Background System
+- [ ] Create `BackgroundManager` class
+  - [ ] Implement level-based backgrounds
+  - [ ] Add background caching system
+  - [ ] Create smooth transitions
+  - [ ] Test background loading performance
+
+## ⚙️ Phase 4: Configuration (Week 7) - MEDIUM PRIORITY
+
+### Configuration System
+- [ ] Implement `ConfigManager` class
+  - [ ] Design configuration storage
+  - [ ] Add file loading/saving
+  - [ ] Implement runtime configuration updates
+  - [ ] Add configuration validation
+  - [ ] Test configuration persistence
+
+- [ ] Migrate constants
+  - [ ] Move window size settings
+  - [ ] Convert gameplay constants
+  - [ ] Update UI layout values
+  - [ ] Set up default configurations
+
+### Settings Management
+- [ ] Create settings persistence
+  - [ ] Save audio preferences
+  - [ ] Store control mappings
+  - [ ] Remember window settings
+  - [ ] Test settings across restarts
+
+## 📡 Phase 5: Event System (Week 8) - MEDIUM PRIORITY
+
+### Event Bus Implementation
+- [ ] Create `EventBus` template class
+  - [ ] Implement subscription system
+  - [ ] Add event publishing mechanism
+  - [ ] Create handler management
+  - [ ] Test event delivery reliability
+
+- [ ] Define Game Events
+  - [ ] Create LevelUp event
+  - [ ] Implement LineCleared event
+  - [ ] Add GameOver event
+  - [ ] Define PieceSpawned event
+  - [ ] Test event system integration
+
+### Audio System Decoupling
+- [ ] Convert audio callbacks to events
+  - [ ] Replace sound callbacks with event handlers
+  - [ ] Implement music management events
+  - [ ] Add audio configuration events
+  - [ ] Test audio system responsiveness
+
+## 🔌 Phase 6: Dependency Injection (Week 9) - LOW PRIORITY
+
+### Service Container
+- [ ] Implement `ServiceContainer` class
+  - [ ] Design service registration
+  - [ ] Add dependency resolution
+  - [ ] Implement service lifecycle
+  - [ ] Test service dependencies
+
+- [ ] Migrate to dependency injection
+  - [ ] Update manager constructors
+  - [ ] Implement service interfaces
+  - [ ] Add service configuration
+  - [ ] Test dependency resolution
+
+## 🧪 Phase 7: Testing & Quality (Week 10) - LOW PRIORITY
+
+### Unit Testing Setup
+- [ ] Set up testing framework
+  - [ ] Configure Google Test or Catch2
+  - [ ] Create test project structure
+  - [ ] Add basic test utilities
+  - [ ] Set up automated test running
+
+- [ ] Write core tests
+  - [ ] Test ApplicationManager lifecycle
+  - [ ] Validate StateManager transitions
+  - [ ] Test InputManager functionality
+  - [ ] Verify AssetManager operations
+  - [ ] Test ConfigManager persistence
+
+### Integration Testing
+- [ ] Create integration test suite
+  - [ ] Test manager interactions
+  - [ ] Validate state transitions
+  - [ ] Test complete game scenarios
+  - [ ] Run performance regression tests
+
+### Code Quality
+- [ ] Add static analysis
+  - [ ] Configure clang-tidy
+  - [ ] Set up cppcheck
+  - [ ] Add memory leak detection
+  - [ ] Run code coverage analysis
+
+## 🔧 Optimization Tasks - ONGOING
+
+### Performance Optimization
+- [ ] Profile rendering performance
+  - [ ] Identify rendering bottlenecks
+  - [ ] Optimize texture usage
+  - [ ] Improve draw call batching
+  - [ ] Test frame rate consistency
+
+- [ ] Memory optimization
+  - [ ] Analyze memory usage patterns
+  - [ ] Optimize asset loading
+  - [ ] Implement memory pooling where needed
+  - [ ] Test for memory leaks
+
+### Code Quality Improvements
+- [ ] Documentation
+  - [ ] Add comprehensive class documentation
+  - [ ] Document API interfaces
+  - [ ] Create usage examples
+  - [ ] Write development guidelines
+
+- [ ] Error Handling
+  - [ ] Implement consistent error handling
+  - [ ] Add logging system
+  - [ ] Create error recovery mechanisms
+  - [ ] Test error scenarios
+
+### Platform Compatibility
+- [ ] Cross-platform testing
+  - [ ] Test on Windows
+  - [ ] Validate Linux compatibility
+  - [ ] Check macOS support
+  - [ ] Test different screen resolutions
+
+## 📊 Progress Tracking
+
+### Metrics to Monitor
+- [ ] **Code Complexity**: Reduce main() from 1,678 to <100 lines
+- [ ] **Test Coverage**: Achieve >90% code coverage
+- [ ] **Build Time**: Maintain fast compilation
+- [ ] **Performance**: No regression in frame rate
+- [ ] **Memory Usage**: Stable memory consumption
+
+### Milestones
+- [ ] **Milestone 1**: Basic managers extracted and working
+- [ ] **Milestone 2**: State system fully refactored
+- [ ] **Milestone 3**: Rendering system modularized
+- [ ] **Milestone 4**: Configuration system implemented
+- [ ] **Milestone 5**: Event system integrated
+- [ ] **Milestone 6**: All tests passing, full documentation
+
+### Success Criteria
+- [ ] Main function reduced to application setup only
+- [ ] All components unit testable
+- [ ] Easy to add new features without modifying core code
+- [ ] Clear separation of concerns throughout codebase
+- [ ] Comprehensive documentation and examples
+
+---
+
+## 📝 Notes
+
+- **Priority levels**: CRITICAL = Must complete, HIGH = Important for maintainability, MEDIUM = Quality improvements, LOW = Nice to have
+- **Estimated timeline**: 10 weeks for complete refactoring
+- **Risk mitigation**: Each phase should maintain working application
+- **Testing strategy**: Test after each major change to ensure no regressions
+- **Documentation**: Update as you go, don't leave it for the end