Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions TODO_SOLID.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# SOLID Principles Analysis & Refactoring TODO

This document outlines the identified violations of SOLID principles within the project's architecture. The issues are prioritized by severity to guide refactoring efforts.

---

## Part 1: CSS Processing Architecture Analysis

### High Severity

#### 1. Violation: Interface Segregation Principle (ISP) & Single Responsibility Principle (SRP)

- **Component:** `StyleAccumulator` interface and `ComputedStyle` class (`src/css/style.ts`).
- **Problem:** The `StyleAccumulator` is a "fat interface" that includes dozens of optional properties for every possible CSS style. Consequently, all `PropertyParser` implementations are forced to depend on this massive interface. The `ComputedStyle` class becomes a "God Object" responsible for managing all style-related data, leading to very low cohesion.
- **Impact:** High coupling, poor maintainability, and difficult testing.
- **Suggested Refactoring:** Decompose `StyleAccumulator` into smaller, cohesive interfaces (`FontStyles`, `BoxModelStyles`, etc.) and refine parser signatures to depend only on the interfaces they need.

---

### Medium Severity

#### 2. Violation: Open/Closed Principle (OCP) & Single Responsibility Principle (SRP)

- **Component:** `registerAllPropertyParsers` function (`src/css/parsers/register-parsers.ts`).
- **Problem:** This is a centralized function that must be modified to add any new CSS property parser. This violates OCP (not closed for modification) and SRP (knows about every parser).
- **Impact:** Extensibility bottleneck and reduced modularity.
- **Suggested Refactoring:** Implement an auto-registration mechanism where parser modules can register themselves, removing the need for a central registration function.

---
---

## Part 2: PDF Generation Architecture Analysis

### High Severity

#### 1. Violation: Dependency Inversion Principle (DIP) & Open/Closed Principle (OCP)

- **Component:** `PagePainter` class (`src/pdf/page-painter.ts`).
- **Problem:** High-level modules like `PagePainter` depend directly on concrete low-level implementations (e.g., `TextRenderer`), instantiating them directly. This violates DIP and OCP, as new renderers cannot be added without modifying `PagePainter`.
- **Impact:** High rigidity, strong coupling, and untestable code.
- **Suggested Refactoring:** Introduce renderer interfaces (e.g., `ITextRenderer`) and use Dependency Injection (DI) to provide concrete implementations to the `PagePainter` constructor.

#### 2. Violation: Single Responsibility Principle (SRP)

- **Component:** `convertNode` function (`src/pdf/layout-tree-builder.ts`).
- **Problem:** This is a "God function" that centralizes the conversion logic for all types of nodes and styles (text, images, backgrounds, shadows, etc.).
- **Impact:** Low cohesion, high complexity, and fragility.
- **Suggested Refactoring:** Decompose the function by responsibility or use a Strategy Pattern where different "Node Converters" can be registered for different node types.

---

### Medium Severity

#### 3. Violation: Interface Segregation Principle (ISP)

- **Component:** `RenderBox` interface (`src/pdf/types.ts`).
- **Problem:** `RenderBox` is a "fat interface" aggregating properties for all possible node types, forcing components to depend on irrelevant data.
- **Impact:** Unnecessary coupling and reduced type safety.
- **Suggested Refactoring:** Refactor `RenderBox` into a discriminated union type (`type RenderNode = ContainerNode | TextNode | ImageNode;`) where each type is specialized.

#### 4. Violation: Single Responsibility Principle (SRP)

- **Component:** `renderPdf` function (`src/pdf/render.ts`).
- **Problem:** The function orchestrates the rendering pipeline but also handles setup logic (font initialization, header/footer layout).
- **Impact:** Reduced cohesion, making the code harder to follow and test.
- **Suggested Refactoring:** Separate configuration from orchestration. Move setup logic into a "context" object that is passed to `renderPdf`.

---
---

## Part 3: Layout Architecture Analysis

### No Violations Found

- **Conclusion:** The layout architecture, centered around the `LayoutEngine` and the `LayoutStrategy` interface (`src/layout/pipeline/`), is an exemplary model of SOLID design within this project.
- **Key Strengths:**
- **SRP:** `LayoutEngine` orchestrates, while individual strategies handle specific layout algorithms.
- **OCP & DIP:** The engine depends on the `LayoutStrategy` abstraction, and concrete strategies are provided via Dependency Injection. This makes the system fully extensible to new layout types without modifying the engine.
- **LSP & ISP:** The `LayoutStrategy` interface is a simple, robust contract that is well-defined and correctly implemented by all strategies.
- **Recommendation:** This architecture should be used as a reference or blueprint for refactoring the CSS and PDF modules. The patterns used here (Strategy Pattern, Dependency Injection) would directly solve many of the issues identified in the other modules.

---
---

## Part 4: Font Architecture Analysis (`src/pdf/font`)

### High Severity

#### 1. Violation: Open/Closed Principle (OCP) & Dependency Inversion Principle (DIP)

- **Component:** The entire font embedding architecture, particularly `FontEmbedder` (`embedder.ts`) and `FontRegistry` (`font-registry.ts`).
- **Problem:** The architecture is rigidly designed for the TTF font format. High-level modules like `FontRegistry` directly instantiate and depend on the concrete low-level `FontEmbedder`, which itself contains TTF-specific logic. There are no abstractions for font parsers or embedders.
- **Impact:**
- **Rigidity:** The system is not open to extension with new font formats (like WOFF2 or OTF) without significant modifications to existing code.
- **High Coupling & Untestable Code:** `FontRegistry` is tightly coupled to `FontEmbedder`, making it very difficult to unit test in isolation.
- **Suggested Refactoring:**
1. **Introduce Abstractions:** Create interfaces like `IFontParser` and `IFontEmbedder`.
2. **Implement Strategy Pattern:** Create concrete classes like `TtfParser` and `TtfEmbedder` that implement these interfaces.
3. **Use Dependency Injection:** Refactor `FontRegistry` to depend on the `IFontEmbedder` interface, and inject the concrete implementation.

---

### Medium Severity

#### 2. Violation: Single Responsibility Principle (SRP)

- **Component:** `FontRegistry` class and `FontEmbedder` class.
- **Problem:**
- `FontRegistry`: Accumulates multiple responsibilities: font caching, font name resolution/aliasing, font variant selection, and orchestration of embedding.
- `FontEmbedder`: Its `embedFont` method is a "God method" that handles the low-level construction of multiple complex PDF objects.
- **Impact:** The classes have low cohesion, making them harder to understand, maintain, and test.
- **Suggested Refactoring:**
- **For `FontRegistry`:** Extract the logic for font name resolution and variant selection into a separate `FontResolver` class.
- **For `FontEmbedder`:** Decompose the `embedFont` method by extracting the logic for creating PDF objects into dedicated builder or factory functions.