Update diagram colors and clarify design pattern documentation#28
Conversation
…rn documentation with side-by-side comparisons
There was a problem hiding this comment.
Pull request overview
Updates documentation/diagrams to better distinguish Strategy vs Factory in the codebase, and adjusts Mermaid styling for the use case diagram.
Changes:
- Clarifies Strategy Pattern documentation with explicit runtime-selection framing and a Strategy vs Factory comparison.
- Expands Factory Pattern documentation with object-construction examples and a side-by-side table.
- Updates Mermaid use case diagram stroke colors to black for improved visual consistency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| diagrams/07_design_patterns.md | Refines Strategy/Factory explanations and adds illustrative examples/comparisons. |
| diagrams/04_use_case_diagram.md | Updates Mermaid node stroke colors to #000000. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| registry := NewDefaultRegistry() // Factory builds the registry | ||
| runner := &DefaultRunner{} // Factory builds the runner | ||
| storage := &LocalTempStorageAdapter{BaseDir: "/dev/shm"} // Factory builds storage | ||
| svc := NewSubmissionService(runner, registry, storage) // Factory wires everything | ||
|
|
||
| // Later at runtime — Strategy selects the algorithm: | ||
| strategy, _ := svc.registry.Get("python") // ← This is Strategy, not Factory | ||
| strategy.Prepare(ws, sourceCode) // ← Strategy executes |
There was a problem hiding this comment.
The example uses &LocalTempStorageAdapter{BaseDir: "/dev/shm"}, but LocalTempStorageAdapter is defined as an empty struct (see backend/processSubmission/local_storage_adapter.go) and has no BaseDir field. As written, this snippet won’t compile and also contradicts the current implementation which always uses os.TempDir(). Update the example to match real construction (e.g., &LocalTempStorageAdapter{}) or change the adapter to support configuring a base dir if that’s the intent.
| registry := NewDefaultRegistry() // Factory builds the registry | |
| runner := &DefaultRunner{} // Factory builds the runner | |
| storage := &LocalTempStorageAdapter{BaseDir: "/dev/shm"} // Factory builds storage | |
| svc := NewSubmissionService(runner, registry, storage) // Factory wires everything | |
| // Later at runtime — Strategy selects the algorithm: | |
| strategy, _ := svc.registry.Get("python") // ← This is Strategy, not Factory | |
| strategy.Prepare(ws, sourceCode) // ← Strategy executes | |
| registry := NewDefaultRegistry() // Factory builds the registry | |
| runner := &DefaultRunner{} // Factory builds the runner | |
| storage := &LocalTempStorageAdapter{} // Factory builds storage | |
| svc := NewSubmissionService(runner, registry, storage) // Factory wires everything | |
| // Later at runtime — Strategy selects the algorithm: | |
| strategy, _ := svc.registry.Get("python") // ← This is Strategy, not Factory | |
| strategy.Prepare(ws, sourceCode) // ← Strategy executes |
No description provided.