Files
spacetris/REFACTORING_ANALYSIS.md
2025-08-17 10:07:34 +02:00

12 KiB

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:

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:

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:

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:

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:

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:

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:

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:

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:

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:

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:

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:

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:

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)

  1. Configuration System - Remove magic numbers
  2. Asset Manager - Centralize resource management
  3. UI Renderer - Separate UI from game rendering
  4. Event System - Decouple components

Low Priority (Quality of Life)

  1. Service Container - Improve dependency management
  2. Logging System - Better debugging support
  3. Performance Profiler - Identify bottlenecks
  4. 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.