layout: true class: typo, typo-selection --- count: false class: nord-dark, middle, center # 🧱 Refactoring with Design Patterns ### Two Polyglot Case Studies — physdes & LDS across C++ · Rust · Python @luk036 👨‍💻 · 2026 📅 · 30 min ⏱️ --- ### 📋 Agenda .pull-left[ **Case Study 1 — physdes** 🧱 **1️⃣ Factory Method** 🏭 - The duplicated node-creation smell - One `_create_node` to rule them all - C++ / Rust / Python before & after **2️⃣ Builder Pattern** 🧰 - Telescoping constructors - Fluent `builder().margin(..).build()` - Adding tests & doctests **3️⃣ Template Method** 🏗️ - Three near-identical decomposers - Policy hooks + shared skeleton ] .pull-right[ **4️⃣ Cross-Language Parity** 🌐 - Same pattern, three idioms - Verification & results **Case Study 2 — LDS family** 🧮 **5️⃣ Template Method · Digit Core · Contracts** 🏗️ - Four siblings, one `value_at` hook - Thread safety & dependents **6️⃣ Lessons Learned** 🎯 - What to refactor, what to leave - Q&A ] --- ### 🎯 Why Refactor with Design Patterns? .pull-left[ **Patterns give code a shape** 🧩 - 🏷️ **Shared vocabulary** — "this is a Factory Method" communicates intent in one word - 🔍 **Spot duplication** — knowing the pattern reveals the *smell* that needs fixing - 🧠 **AI-friendly** — "refactor as a Builder" beats "clean this up" every time - 📖 **Self-documenting** — `_create_node` tells you how nodes come into existence ] .pull-right[ **But patterns are not magic** ⚠️ - 🚫 **Don't force them** — a Visitor for a 30-line tree is over-engineering - 🎯 **Match the idiom** — Rust's `..Default::default()` already *is* a builder - ⚖️ **YAGNI** — refactor real duplication, not hypothetical futures - ✅ **Verify behavior** — 12,000+ assertions must still pass after the rewrites ] --- ### 🌐 The physdes Polyglot Family Three siblings implementing the same VLSI physical-design algorithms: .mermaid[ <pre> graph LR CPP["🧱 physdes-cpp\nC++20 header-only"] --> ALGO["⚙️ Shared Algorithms\nDME · Global Router\nRPolygon Cut"] RS["🦀 physdes-rs\nRust crate"] --> ALGO PY["🐍 physdes-py\nPython package"] --> ALGO ALGO --> TEST["✅ Cross-Language\nVerification"] style CPP fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style RS fill:#e3f2fd,stroke:#1565c0,color:#2e3440,stroke-width:3px style PY fill:#fff9c4,stroke:#f57f17,color:#2e3440,stroke-width:3px style ALGO fill:#e8eaf6,stroke:#283593,color:#2e3440,stroke-width:3px style TEST fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px </pre> ] **The mission:** refactor *all three* with the same three patterns, idiomatically. --- ### 🔍 The Three Smells We Found | Smell | Where | Pattern to Apply | |-------|-------|------------------| | 🏭 **5× duplicated node creation** | `global_router` (C++/Rust/Py) | **Factory Method** | | 🧰 **8-arg telescoping constructor** | `ClockTreeVisualizer` (all three) | **Builder** | | 🏗️ **3× near-identical recursive skeletons** | `rpolygon_cut` (C++/Py) | **Template Method** | Each smell is *recognizable precisely because* we know the target pattern. --- class: nord-light, middle, center ## 🏭 Part 1 — Factory Method (physdes) --- ### 🏭 The Smell: Node Creation Scattered Everywhere In `GlobalRoutingTree`, **5 different methods** hand-rolled the same dance: ```cpp // insert_steiner_node std::string steiner_id = "steiner_" + std::to_string(this->next_steiner_id++); this->_arena.emplace_back(steiner_id, NodeType::Steiner, point); RoutingNode* node = &this->_arena.back(); this->nodes[steiner_id] = node; // insert_terminal_node → same, with "terminal_" prefix // insert_node_on_branch → if/else on enum to pick prefix // _insert_terminal_impl → both terminal AND steiner // constructor → hard-coded "source" ``` - 🔁 ID generation + counter + arena + map registration, repeated 5× - 🐛 `insert_node_on_branch` had a latent bug: empty ID if given `NodeType::Source` --- ### 🏭 The Fix: One Factory Method ```cpp // C++ RoutingNode<IntPoint>* GlobalRoutingTree<IntPoint>::_create_node( NodeType type, const IntPoint& pt) { std::string id; switch (type) { case NodeType::Steiner: id = "steiner_" + std::to_string(next_steiner_id++); break; case NodeType::Terminal: id = "terminal_" + std::to_string(next_terminal_id++); break; case NodeType::Source: id = "source"; break; } _arena.emplace_back(id, type, pt); RoutingNode* node = &_arena.back(); nodes[id] = node; return node; } ``` All 5 call sites collapse to: `this->_create_node(NodeType::Steiner, point);` 🎯 --- ### 🏭 Same Pattern, Three Idioms .pull-left[ **C++** — private method, pointer return: .font-sm[ ```cpp auto _create_node(NodeType type, const IntPoint& pt) -> RoutingNode<IntPoint>*; ``` ] **Rust** — private method, index return: .font-sm[ ```rust fn _create_node(&mut self, node_type: NodeType, pt: Point) -> usize { let id = match node_type { NodeType::Steiner => { let s = format!("steiner_{}", self.next_steiner_id); self.next_steiner_id += 1; s } NodeType::Terminal => { /* ... */ } NodeType::Source => "source".to_string(), }; self.add_node(RoutingNode::new(&id, node_type, pt)) } ``` ] ] .pull-right[ **Python** — method returning the node object: .font-sm[ ```python def _create_node(self, node_type, pt): if node_type == NodeType.Steiner: node_id = f"steiner_{self.next_steiner_id}" self.next_steiner_id += 1 elif node_type == NodeType.Terminal: node_id = f"terminal_{self.next_terminal_id}" self.next_terminal_id += 1 else: # Source node_id = "source" node = RoutingNode(node_id, node_type, pt) self.nodes[node_id] = node return node ``` ] **One factory, zero duplication, three languages.** 🌐 ] --- ### 🏭 Subtle Behaviors We Had to Preserve Python taught us the factory isn't a *mechanical* extraction: - ⚠️ **`insert_terminal_node`** — the node must NOT be registered before `_find_nearest_node` runs, or it becomes its own nearest neighbor (distance 0) → reordered: *resolve parent first, then create* - ⚠️ **`insert_node_on_branch`** — a test asserted it raises `ValueError` for `NodeType.Source` → kept the guard in front of the factory **Lesson:** patterns remove duplication, not contracts. Tests are the safety net. 🧪 --- class: nord-light, middle, center ## 🧰 Part 2 — Builder Pattern (physdes) --- ### 🧰 The Smell: Telescoping Constructor ```python # Python — 8 positional parameters with defaults class ClockTreeVisualizer: def __init__(self, margin=50, node_radius=8, wire_width=2, sink_color="#4CAF50", internal_color="#2196F3", root_color="#F44336", wire_color="#666666", text_color="#333333"): ... ``` - 🤯 Call sites like `ClockTreeVisualizer(60, 10, 3, "#2E7D32", ...)` — what is `60`? - ➕ Adding a 9th option touches every call site - 📖 Hard to read, hard to extend, hard to review --- ### 🧰 The Fix: Fluent Builder .font-sm[ ```python # Python viz = (ClockTreeVisualizer.builder() .margin(60) .node_radius(10) .wire_width(3) .sink_color("#2E7D32") .build()) ``` ```cpp // C++ — same intent auto viz = ClockTreeVisualizer::Builder() .margin(60) .node_radius(10) .wire_width(3) .build(); ``` ```rust // Rust — same intent let viz = ClockTreeVisualizer::builder() .margin(60) .node_radius(10) .wire_width(3) .build(); ``` ] Self-documenting: each option is *named* at the call site. ✨ --- ### 🧰 Language Idioms for the Builder .pull-left[ **C++** — private ctor, nested Builder: ```cpp class ClockTreeVisualizer { public: class Builder { /* setters + build() */ }; private: friend class Builder; ClockTreeVisualizer(int margin, ...); // private }; ``` Forces every customization through the Builder. 🔒 **Python** — builder is a *classmethod*: ```python @classmethod def builder(cls): return ClockTreeVisualizerBuilder() ``` ] .pull-right[ **Rust** — the *honest* note 🦀: Rust already had the idiomatic answer: ```rust let viz = ClockTreeVisualizer { margin: 40, node_radius: 6, ..Default::default() // struct-update syntax }; ``` We added a real `ClockTreeVisualizerBuilder` anyway for **parity with the siblings** — but Rust's native ergonomics make it optional. **Lesson:** sometimes the pattern is *already the language*. 🎯 ] --- ### 🧰 Tests Guard the Builder Every language got builder tests: ``` ✅ Builder Configuration — custom values reach the output ✅ Builder Defaults Match — builder().build() == default ctor ✅ Builder Is Fluent — setters return self/builder ``` Plus a **doctest** in the Python class docstring (runs under pytest): ``` >>> viz = ClockTreeVisualizerBuilder().margin(10).node_radius(5).build() >>> viz.margin 10 ``` **Patterns without tests are just opinions.** 🧪 --- class: nord-light, middle, center ## 🏗️ Part 3 — Template Method (physdes) --- ### 🏗️ The Smell: Three Near-Identical Decomposers `rpolygon_cut` splits a rectilinear polygon into convex pieces — **three variants** (convex / explicit / implicit) shared ~80% of their code: ``` base cases ──► find concave point ──► min-dist scan ──► re-wire ──► recurse ──► merge │ │ │ │ │ │ ✓ differs ✓ differs ✓ differs ✓ differs │ (3 variants) (2 variants) (2 variants) (2 variants) │ └── convex had a 3-node base case; explicit/implicit didn't ``` Only the *hooks* differed — the **skeleton was identical**. 🕸️ --- ### 🏗️ The Fix: Extract the Skeleton, Inject the Hooks .pull-left[ **Before — 3× ~80-line recursive functions**: .font-sm[ ``` rpolygon_cut_convex_recur 295 duplicated lines rpolygon_cut_explicit_recur rpolygon_cut_implicit_recur ``` ] ] .pull-right[ **After — one skeleton + policy hooks**: .font-sm[ ``` rpolygon_cut_recur_impl ← Template Method ├── ConvexCutPolicy ← find_concave_point + triangle base case ├── ExplicitCutPolicy ← find_concave_point only └── ImplicitCutPolicy ← different scan + rewiring + target ``` ] ] **Net −82 lines** in the C++ file. 📉 **Python version** — same idea, function-parameter style: ```python def rpolygon_cut_recur(v1, lst, is_anticlockwise, rdll, find_concave_point, has_triangle_base_case): # ... shared skeleton ... vcurr = find_concave_point(v1, ...) # ← hook def rpolygon_cut_convex_recur(...): return rpolygon_cut_recur(..., _find_convex_concave_point, True) def rpolygon_cut_explicit_recur(...): return rpolygon_cut_recur(..., _find_explicit_concave_point, False) ``` --- ### 🏗️ The Template Method at a Glance .mermaid[ <pre> graph TD SKEL["🏗️ rpolygon_cut_recur_impl\nShared Skeleton\nbase cases · fallback · insert · recurse"] SKEL --> H1["🎣 Hook: find_concave_point"] SKEL --> H2["🎣 Hook: find_min_dist_point"] SKEL --> H3["🎣 Hook: insert_cut (rewiring)"] SKEL --> H4["🎣 Hook: first_target (recursion)"] H1 --> C1["🟢 ConvexCutPolicy\ndirection-reversal test"] H1 --> C2["🔵 ExplicitCutPolicy\narea-sign test"] H1 --> C3["🟠 ImplicitCutPolicy\ncorner-point test"] H2 --> C1 H2 --> C3 H3 --> C1 H3 --> C3 H4 --> C3 style SKEL fill:#e8eaf6,stroke:#283593,color:#2e3440,stroke-width:3px style C1 fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style C2 fill:#e3f2fd,stroke:#1565c0,color:#2e3440,stroke-width:3px style C3 fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px </pre> ] **Rust note:** its `rpolygon_cut` was a *placeholder stub* — all three "variants" were byte-identical and returned the whole polygon. We deduplicated only the wrapper boilerplate, and left the stubs as future extension points. 🦀 --- ### 🏗️ Behavioral Equivalence — the Hard Part The C++ refactor looked trivial but had **four hidden contracts**: | Difference | How preserved | |------------|---------------| | 🔺 Convex's 3-node triangle base case | `has_triangle_base_case = true` flag | | 🎯 Implicit recurses on `v_min`, not `vcurr` | `first_target` hook | | 📏 Implicit scans from a corner point | `find_min_dist_point` hook | | 🔁 Implicit's different list rewiring | `insert_cut` hook | **Result:** all **701 assertions** still passed with zero behavioral drift. ✅ --- class: nord-light, middle, center ## 🌐 Part 4 — Cross-Language Parity (physdes) --- ### 🌐 Same Pattern, Three Codebases .mermaid[ <pre> graph TD subgraph Factory["🏭 Factory Method"] F1["C++ _create_node\n5 sites → 1"] F2["Rust _create_node\n5 sites → 1"] F3["Py _create_node\n5 sites → 1"] end subgraph Builder["🧰 Builder"] B1["C++ nested Builder\nprivate ctor"] B2["Rust Builder struct\n+ Default"] B3["Py Builder class\n+ classmethod"] end subgraph Template["🏗️ Template Method"] T1["C++ policy hooks\n−82 lines"] T2["Rust shared wrapper\n(stub algorithms)"] T3["Py finder-hook skeleton\n2 variants → 1"] end style Factory fill:#e3f2fd,stroke:#1565c0,color:#2e3440,stroke-width:3px style Builder fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style Template fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px </pre> ] All pushed to GitHub in **9 commits** (3 per repo) on the `dev` branches. 🚀 --- ### 🌐 Verification Across All Three | Project | Test count | Toolchain gates | |---------|-----------|-----------------| | 🧱 **physdes-cpp** | 132 cases · 701 assertions | `-W4 -WX`, clang-format | | 🦀 **physdes-rs** | 299 lib + 11 integ + 66 doc | `cargo fmt/clippy/doc -- -D warnings` | | 🐍 **physdes-py** | 600 passed · 1 skipped | black · isort · flake8 | Every gate **green** before any commit. ✅ --- class: nord-light, middle, center ## 🧮 Part 5 — Case Study 2: The LDS Family --- ### 🧮 The LDS Polyglot Family Four siblings implementing the same low-discrepancy sequence generators (Van der Corput · Halton · Circle · Sphere): .mermaid[ <pre> graph TD CPP["🧱 lds-cpp\nC++20 templates · constexpr"] --> CORE["🔢 Shared Core\nvdc_digit_sum"] CC["⚙️ lds-gen-cpp\nC++20 runtime · std::atomic"] --> CORE RS["🦀 lds-rs\nRust crate"] --> CORE PY["🐍 lds-gen\nPython package"] --> CORE CORE --> PROTO["🎯 Uniform Protocol\npop · peek · reseed · get_index"] style CPP fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style CC fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style RS fill:#e3f2fd,stroke:#1565c0,color:#2e3440,stroke-width:3px style PY fill:#fff9c4,stroke:#f57f17,color:#2e3440,stroke-width:3px style CORE fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px style PROTO fill:#e8eaf6,stroke:#283593,color:#2e3440,stroke-width:3px </pre> ] **The mission:** refactor *all four* with the same patterns, idiomatically — and keep every public API byte-for-byte compatible. --- ### 🔍 The Smells We Found (LDS) | Smell | Where | Fix | |-------|-------|-----| | 🏗️ **~250 lines of identical protocol boilerplate** (pop/peek/skip/reseed/begin/end) | 6 template classes in `lds-cpp` | **Template Method** (CRTP base) | | 🔢 **Digit-reversal loop inlined in `pop()` *and* `peek()`** | every `VdCorput` (all 4 languages) | **Shared digit core** | | 🐍 **Same `pop`/`reseed`/iterators/batch duplicated** | 7 classes in Python `lds.py` | **Generic base class** | | 🏭 **11-case factory switch** | `HaltonN::create_vdc` (lds-cpp) | **Factory registry table** | --- ### 🧮 Template Method — State / Computation Separation Every generator fused two concerns into `pop()`: - 🧮 **the computation** — "what is the value at index *n*?" - 🔢 **the state** — "how far have I advanced?" ``` pop() → count += 1; compute(count) peek() → compute(count + 1) ← same loop, duplicated after: pop() = value_at(++count) peek() = value_at(count + 1) ← consistent BY CONSTRUCTION ``` Because `value_at` is pure, the components inside composites become stateless calculators. ✅ --- ### 🧮 C++ — Template Method via CRTP .font-sm[ ```cpp // lds-gen-cpp — the protocol lives once in the base template <typename Derived, typename Value> class GeneratorBase { public: auto pop() -> Value { auto n = count_.fetch_add(1, std::memory_order_relaxed) + 1; return derived().value_at(n); // ← hook } auto peek() -> Value { return derived().value_at(count_.load(std::memory_order_relaxed) + 1); } // skip / reseed / get_index … protected: std::atomic<unsigned long> count_{0}; // state lives here private: auto derived() -> Derived& { return static_cast<Derived&>(*this); } }; class Halton : public GeneratorIterable<Halton, std::array<double, 2>> { VdCorput vdc0, vdc1; public: auto value_at(unsigned long n) const -> std::array<double, 2> { return {vdc0.value_at(n), vdc1.value_at(n)}; // ← the ONLY thing left } }; ``` ] `lds-cpp` uses a plain `unsigned long` (constexpr priority); `lds-gen-cpp` uses `std::atomic` (thread-safe priority). Same skeleton. 🧱 --- ### 🦀 Rust — a Trait with Default Methods .font-sm[ ```rust // lds-rs — the Generator trait (Template Method) pub trait Generator { type Value; fn counter(&self) -> &AtomicU64; fn value_at(&self, n: u64) -> Self::Value; // ← hook fn pop(&mut self) -> Self::Value { let n = self.counter().fetch_add(1, Ordering::Relaxed) + 1; self.value_at(n) } fn peek(&self) -> Self::Value { self.value_at(self.counter().load(Ordering::Relaxed) + 1) } // advance / get_index / reseed defaults … } ``` ] A `macro_rules!` keeps the protocol methods **inherent**, so the public API is unchanged (no trait import needed to call `pop()`): .font-sm[ ```rust impl_generator_protocol!(Halton, [f64; 2]); // pop/peek/advance/… + Iterator ``` ] > ⚠️ Rust trait methods are *not* automatically inherent — that is why the > macro exists. Public API preservation drives the design. 🦀 --- ### 🐍 Python — a Generic Base Class ```python # lds-gen — GeneratorBase[T] (Template Method) class GeneratorBase(Generic[T]): def pop(self) -> T: with self._count_lock: self._count += 1 return self.value_at(self._count) # ← hook def value_at(self, n: int) -> T: raise NotImplementedError # __iter__ / __next__ / pop_batch / iter_batch … class Halton(GeneratorBase[List[float]]): def value_at(self, n: int) -> List[float]: return [self.vdc0.value_at(n), self.vdc1.value_at(n)] ``` - `T` is the value type — `float`, `List[float]`, `int`, … - All 7 classes shed ~25 lines of identical boilerplate each 📉 --- ### 🧮 Template Method at a Glance .mermaid[ <pre> graph TD BASE["🧩 GeneratorBase · Generator trait\npop() → value_at(++count)\npeek() → value_at(count + 1)\nskip · reseed · get_index"] HOOK["🎣 Hook: value_at(n)\npure · no state change"] V["VdCorput\nvdc(n, base)"] H["Halton\n[vdc0(n), vdc1(n)]"] C["Circle\ncos/sin mapping"] S["Sphere3Hopf\nHopf mapping"] BASE --> HOOK HOOK --> V HOOK --> H HOOK --> C HOOK --> S style BASE fill:#e8eaf6,stroke:#283593,color:#2e3440,stroke-width:3px style HOOK fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px style V fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style H fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style C fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style S fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px </pre> ] **One skeleton, many generators.** 🎯 --- ### 🔢 Shared Digit Core — One Loop, Four Languages The radical inverse is the heart of every generator: $$ \phi_b(n) = \sum_{k=0}^{m} d_k \, b^{-k-1} $$ Every `VdCorput::pop()` *and* `peek()` inlined the same base-`b` digit loop. Extract it once: .font-sm[ ```cpp template <typename T, typename Table> constexpr auto vdc_digit_sum(unsigned long n, unsigned long base, const Table& weights) -> T { T reslt{}; std::size_t idx = 0; while (n != 0) { const auto remainder = n % base; n /= base; reslt += static_cast<T>(remainder) * weights[idx++]; } return reslt; } ``` ] - 🦀 **Rust** — generic over `f64`/`u64` via a small `DigitValue` trait (`f64` has no `From<u64>` — lossy) - 🐍 **Python** — the existing `vdc()` (float) plus a brand-new public `vdc_i()` (int) Three implementations of the same loop became **one**. 🔁 → 1️⃣ --- ### 📜 The Formal Contract — Contract as Code Each language now *enforces* the protocol at compile time: ```cpp // C++20 — the concept (lds-cpp / lds-gen-cpp) template <typename G, typename V> concept SequenceGenerator = requires(G g, unsigned long n) { { g.pop() } -> std::convertible_to<V>; { g.peek() } -> std::convertible_to<V>; g.skip(n); g.reseed(n); { g.get_index() } -> std::convertible_to<unsigned long>; }; static_assert(SequenceGenerator<VdCorput, double>); // compile-time check ``` - 🦀 **Rust** — the `Generator` trait *is* the contract - 🐍 **Python** — `GeneratorBase[Generic[T]]` + type hints - ⚙️ **lds-gen-cpp** — the same concept also holds for the runtime `VdCorputBase` / `HaltonN` — both families interchangeable --- ### 🌐 LDS Cross-Language Parity .mermaid[ <pre> graph TD subgraph CPP1["🧱 lds-cpp"] A1["GeneratorBase CRTP\nconstexpr · concept · mixin"] end subgraph CPP2["⚙️ lds-gen-cpp"] A2["GeneratorBase CRTP\nstd::atomic counter"] end subgraph RS["🦀 lds-rs"] A3["Generator trait\ndefault methods + macro"] end subgraph PY["🐍 lds-gen"] A4["GeneratorBase Generic[T]\nlock-guarded _count"] end A1 --> CON["🎯 Same Template Method\none value_at(n) hook"] A2 --> CON A3 --> CON A4 --> CON style CPP1 fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style CPP2 fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style RS fill:#e3f2fd,stroke:#1565c0,color:#2e3440,stroke-width:3px style PY fill:#fff9c4,stroke:#f57f17,color:#2e3440,stroke-width:3px style CON fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px </pre> ] **Public API is unchanged** in every repo — only *additive* surface (`value_at`, the trait/base, `vdc_i`). 🌐 --- ### 🛡️ Thread Safety & Where the Counter Lives The same pattern is *language-shaped*: .mermaid[ <pre> graph TD subgraph CPP["🧱 C++ — counter lives in the base"] B1["GeneratorBase\ncount_ member · inherited by all"] end subgraph RS["🦀 Rust — traits can't hold data"] R1["count: AtomicU64\nfield per struct"] R2["counter() accessor hook"] end B1 --> SAME["1 atomic per generator\nsame semantics"] R1 --> SAME style CPP fill:#c8e6c9,stroke:#2e7d32,color:#2e3440,stroke-width:3px style RS fill:#e3f2fd,stroke:#1565c0,color:#2e3440,stroke-width:3px style SAME fill:#ffe0b2,stroke:#e65100,color:#2e3440,stroke-width:3px </pre> ] Centralizing the counter made composite `pop()` **atomic as a point**: `Halton.pop() = value_at(++count)` — ONE atomic claim, always a valid point. ✅ - 🐍 **lds-gen** — every generator now lock-guarded → **all thread-safe** - 🧱 **lds-cpp** — plain `unsigned long` on purpose: `constexpr` beats thread-safety there (a documented design trade-off) --- ### 🛡️ Do the LDS Dependents Break? We audited every consumer before touching a header: | Dependent | Consumes | Impact | |-----------|----------|--------| | 🍵 **ginger-cpp** | `lds-cpp` (CPM v1.2.5) | Uses only `VdCorput::pop()` (constexpr) + `TWO_PI` → **safe, recompile only** | | 🧱 **physdes-cpp** | `lds-gen-cpp` (not lds-cpp!) | **Unaffected** — no transitive dependency | | 🔮 **sphere-n-cpp** | `lds-gen-cpp` | **Unaffected** | Only **one** of the three "dependents" actually depends on lds-cpp. **Know your dependency graph before you refactor.** 🗺️ --- ### ✅ LDS Verification Across Four Repos | Repo | Tests | Toolchain gates | |------|-------|-----------------| | 🧱 **lds-cpp** | 96 cases · 4,622 assertions | MSVC `/W4 /WX` · clang-format | | ⚙️ **lds-gen-cpp** | 76 cases · 6,405 assertions | `/W4 /WX` · clang-format | | 🦀 **lds-rs** | 97 unit + 12 doc tests | `cargo fmt` · `clippy` · `test` | | 🐍 **lds-gen** | 103 pytest + 19 doctests | black · isort · flake8 | **Every gate green, zero behavioral drift** — exact-value tests matched the Python reference bit-for-bit. ✅ --- class: nord-light, middle, center ## 🎯 Part 6 — Lessons Learned --- ### 🎯 What Refactoring *Really* Means .pull-left[ **Do refactor:** ✅ - 🏭 Real duplication (5× node creation) - 🧰 Telescoping constructors - 🏗️ ~80% identical algorithm skeletons - 📚 When siblings share the same patterns **Don't force it:** 🚫 - 🦀 Rust's struct-update syntax already *is* a builder - 🧩 A Visitor framework for a 30-line internal tree - 🗿 Splitting a cohesive 500-line value type - 🔮 Hypothetical future needs (YAGNI) ] .pull-right[ **Process that made it safe:** 🛡️ 1. 🔍 **Map the smells** — explore agents + code review 2. 🎯 **Name the target pattern** before touching code 3. 🧪 **Baseline tests** before any edit 4. 🔒 **Preserve contracts** — hidden ordering, error guards 5. ✅ **Re-run every gate** — build, lint, format, doc, test 6. 🧱 **Atomic commits** — one pattern per commit ] --- ### 🎯 Key Takeaways - 🧩 **Design patterns are refactoring targets** — recognize the smell, name the pattern, apply it - 🌐 **Polyglot refactoring** keeps sibling projects structurally consistent, making cross-language maintenance easier - 🦀 **Match the idiom** — the same pattern looks different in C++, Rust, and Python - 🧪 **Tests are the safety net** — 12,000+ assertions across the repos never blinked - 🤖 **AI amplification** — telling an agent "apply the Builder pattern" yields far better output than "clean this up" - 📉 **Less code, same behavior** — hundreds of duplicated lines removed, zero behavioral drift --- ### 🗂️ Session Log .pull-left[ **Case Study 1 — physdes** (9 commits) .font-sm[ | Repo | Commits | |------|---------| | 🧱 physdes-cpp | `7942ac7` Factory · `9d8f9be` Builder · `52ac44f` Template Method | | 🦀 physdes-rs | `1bbbeb0` Factory · `01179f1` Builder · `c3b9045` shared wrapper | | 🐍 physdes-py | `2e03732` Factory · `311848a` Builder · `3666c82` Template Method | ] All on `dev`, all gates green, ready for PRs. 📤 ] .pull-right[ **Case Study 2 — LDS family** (~600 lines removed) .font-sm[ | Repo | Δ lines | Highlights | |------|---------|------------| | 🧱 lds-cpp | `lds.hpp` 978 → 802 | CRTP + concept + digit core + factory registry | | ⚙️ lds-gen-cpp | `lds.hpp` 949 → 734 | atomic CRTP · composites atomic-as-point | | 🦀 lds-rs | −75 + 33× `#[inline]` | trait + macro · `DigitValue` | | 🐍 lds-gen | `lds.py` 574 → 430 | `GeneratorBase[T]` · new `vdc_i()` | ] Uncommitted working trees, ready for review. 📤 ] --- ### 📚 Further Reading .pull-left[ **Patterns** - 🧩 _Design Patterns_ — Gamma, Helm, Johnson, Vlissides - 🏗️ _Effective C++ / Modern C++_ — Scott Meyers - 🦀 _Rust Design Patterns_ — rust-unofficial **Case Study 1 — physdes** - 🧱 github.com/luk036/physdes-cpp - 🦀 github.com/luk036/physdes-rs - 🐍 github.com/luk036/physdes-py **Case Study 2 — LDS family** - 🧱 github.com/luk036/lds-cpp - ⚙️ github.com/luk036/lds-gen-cpp - 🦀 github.com/luk036/lds-rs - 🐍 github.com/luk036/lds-gen ] .pull-right[ **Refactoring practice** - 🛠️ _Refactoring_ — Martin Fowler - 📐 _Working Effectively with Legacy Code_ — Michael Feathers - 🧪 Property-based testing (RapidCheck / proptest / Hypothesis) - 🤖 Pattern-aware AI prompting (Sisyphus workflow) **Try it:** pick your own 5-site duplication and give it a factory. 😉 ] --- count: false class: nord-dark, middle, center # 🙏 Thank You! ### Refactoring with Design Patterns @luk036 👨‍💻 · Questions welcome 🎤