diff --git a/ideas/split-loop-with-lambda/storyboard/storyboard.md b/ideas/split-loop-with-lambda/storyboard/storyboard.md new file mode 100644 index 0000000..688da93 --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/storyboard.md @@ -0,0 +1,352 @@ +# Split Loop with Lambda — storyboard / handout + +Step-by-step mechanics of the **Split Loop with Lambda** refactoring, one diagram per +step, each step followed by the exact code (full before/after, highlights as `←` +comments). Written to be sliced into slides later; until then it works as a handout. + +Sources: `../zeroth-draft-of-refactoring-pattern.md` (the mechanics), +`../split-loop-with-lambda-learning-hour.md` (where we stop: after step 3 + the test), +`../exercise/rehearsal-findings.md` (the drift anti-pattern and the "both calls!" +caution). Java only. Pristine start: `../exercise/java`, end state: `../exercise/solution`. + +## How to read the diagrams + +![](svg/legend.svg) + +--- + +## Step 0 — Where we start: one method, three loops, one unwanted dependency + +**Goal:** see the problem before touching anything. `longRangeForecast()` resets, looks +up weather, and scores — and the weather part calls `scorer`, which stands for an +external weather service (it uses `Random`, so the approval test is red *and* flaky). + +![](svg/00-pristine.svg) + +**Before:** + +```java +public Map longRangeForecast() { + var result = new HashMap(); + for (IceCream flavour : IceCream.values()) { + result.put(flavour, 0); + } + List expectedWeather = new ArrayList<>(); + for (Instant interestingDate : DATES) { + scorer.updateSelection(); // ← unwanted dependency (void) + long daysForward = ChronoUnit.DAYS.between(ANCHOR_DATE, interestingDate); + Boolean weather = scorer.lookupWeather(daysForward); // ← unwanted dependency (value) + expectedWeather.add(weather); + } + for (IceCream flavour : result.keySet()) { + result.put(flavour, result.get(flavour) + 10); + var sunnyHolidays = expectedWeather.stream().filter(s -> s).count(); + if (sunnyHolidays > 2 && flavour == IceCream.Vanilla) { + result.put(flavour, result.get(flavour) + 5); + } + if (sunnyHolidays > 1 && expectedWeather.get(1)) { + result.put(flavour, result.get(flavour) + 2); + } + } + return result; +} +``` + +**Checkpoint:** + +- Run the approval test. It fails or flakes. Why? (`Scorer` calls `Random` — it *is* + the unwanted dependency.) +- Plain **Split Loop** can identify the three loops — but it cannot cut the weather + loop loose, because the thing you would introduce and slide is not a value, it is a + *call on an object you don't want*. That is where this pattern starts. + +--- + +## Step 1 — Extract the dependency-using code into a lambda + +**Goal:** make the laziness explicit. Instead of asking the weather service now, we +inject *a function that can answer the weather question when asked*. Both `scorer` +calls go inside the lambda — the void one too (see the anti-pattern slide for what +happens if you leave it behind). This step is a small manual edit: no refactoring tool +does it. + +![](svg/01-extract-lambda.svg) + +**After:** + +```java +public Map longRangeForecast() { + var result = new HashMap(); + for (IceCream flavour : IceCream.values()) { + result.put(flavour, 0); + } + List expectedWeather = new ArrayList<>(); + LongFunction dayOffsetToWeather = daysForward -> { // ← NEW: the lambda... + scorer.updateSelection(); // ← ...holds BOTH scorer calls + return scorer.lookupWeather(daysForward); + }; + for (Instant interestingDate : DATES) { + long daysForward = ChronoUnit.DAYS.between(ANCHOR_DATE, interestingDate); + expectedWeather.add(dayOffsetToWeather.apply(daysForward)); // ← invocation replaced lookupWeather + } + for (IceCream flavour : result.keySet()) { + // ... scoring loop unchanged ... + } + return result; +} +``` + +(IDE will offer to add `import java.util.function.LongFunction;` — take it.) + +**Checkpoint:** + +- Compiles; behaviour unchanged; test still flaky — the lambda still calls `Scorer` + for now, that is fine. +- The lambda's parameter (`daysForward`) is *only* in scope inside the lambda. The + loop still computes its own `daysForward` and hands it to the invocation. + +--- + +## Step 2 — Slide the lambda definition to the top of the method + +**Goal:** *slide, don't move.* The **definition** of the lambda slides to the top of +the method; the **invocation** (`.apply(...)`) stays exactly where the code used to +be. This is the Split Loop "slide statement" move — except what slides is a +function, not a value. + +![](svg/02-slide-lambda.svg) + +**After:** + +```java +public Map longRangeForecast() { + LongFunction dayOffsetToWeather = daysForward -> { // ← SLID to the top... + scorer.updateSelection(); + return scorer.lookupWeather(daysForward); + }; + var result = new HashMap(); + for (IceCream flavour : IceCream.values()) { + result.put(flavour, 0); + } + List expectedWeather = new ArrayList<>(); + for (Instant interestingDate : DATES) { + long daysForward = ChronoUnit.DAYS.between(ANCHOR_DATE, interestingDate); + expectedWeather.add(dayOffsetToWeather.apply(daysForward)); // ← ...invocation stays here + } + for (IceCream flavour : result.keySet()) { + // ... scoring loop unchanged ... + } + return result; +} +``` + +**Checkpoint:** + +- Compiles; behaviour unchanged; test still flaky. +- Read the method out loud: everything it needs from outside the weather loop is + now "given a day offset, is it sunny?" — the loop body itself no longer mentions + `Scorer`. + +--- + +## Step 3 — Extract the weather loop; the lambda becomes a parameter + +**Goal:** select from the `expectedWeather` declaration through the end of the +weather loop and Extract Method. The IDE infers the parameter type +(`LongFunction`) from the lambda variable. The extracted method has no +field dependencies left — `DATES` and `ANCHOR_DATE` are constants — so it can be +`static` right away. + +![](svg/03-extract-method.svg) + +**Before:** end state of step 2 (above). + +**After:** + +```java +public Map longRangeForecast() { + LongFunction dayOffsetToWeather = daysForward -> { + scorer.updateSelection(); + return scorer.lookupWeather(daysForward); + }; + var result = new HashMap(); + for (IceCream flavour : IceCream.values()) { + result.put(flavour, 0); + } + List expectedWeather = collectExpectedWeather(dayOffsetToWeather); // ← EXTRACTED + for (IceCream flavour : result.keySet()) { + result.put(flavour, result.get(flavour) + 10); + var sunnyHolidays = expectedWeather.stream().filter(s -> s).count(); + if (sunnyHolidays > 2 && flavour == IceCream.Vanilla) { + result.put(flavour, result.get(flavour) + 5); + } + if (sunnyHolidays > 1 && expectedWeather.get(1)) { + result.put(flavour, result.get(flavour) + 2); + } + } + return result; +} + +private static List collectExpectedWeather( // ← NEW — and already static + LongFunction dayOffsetToWeather) { + var expectedWeather = new ArrayList(); + for (Instant interestingDate : DATES) { + long daysForward = ChronoUnit.DAYS.between(ANCHOR_DATE, interestingDate); + expectedWeather.add(dayOffsetToWeather.apply(daysForward)); + } + return expectedWeather; +} +``` + +**Checkpoint — the drift test.** This is where the refactoring can silently fail: + +- `grep -n scorer src/main/java/codingdojo/LongRangeForecasts.java` — the new method + must contain **no** hits. +- Try to make `collectExpectedWeather` `static` (it already is here). If any field + reference remained, it would not compile. +- If `scorer.updateSelection()` is still in the loop instead of inside the lambda, + the dependency travelled *with* the extraction and the split is cosmetic. Put it + in the lambda and start this step again. + +### Inset — the overload: let the λ come from outside + +**Goal:** right now the lambda is a local variable — the test still cannot reach the +seam. Change Signature on the big method: *add* the lambda as a parameter and *drop* +the local. Then give callers a no-arg overload that bakes in the "real" lambda. + +![](svg/03b-overload.svg) + +**After:** + +```java +public Map longRangeForecast() { // ← no-arg overload: callers unchanged + return longRangeForecast(daysForward -> { + scorer.updateSelection(); + return scorer.lookupWeather(daysForward); + }); +} + +public Map longRangeForecast( // ← the seam: λ is a parameter + LongFunction dayOffsetToWeather) { + var expectedWeather = collectExpectedWeather(dayOffsetToWeather); + // ... reset loop + scoring loop, unchanged ... + return result; +} +``` + +**Checkpoint:** compiles; existing callers (and the old flaky test, if you keep it) +still work through the no-arg overload. + +--- + +## Step 5-lite — test through the seam with a pure lambda + +**Goal:** replace the weather service with a pure function in the test. Deterministic, +glanceable, and `Scorer` is never touched. + +![](svg/04-pure-test.svg) + +**The test (end of document version — this is the whole thing):** + +```java +package codingdojo; + +import org.approvaltests.Approvals; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Map; +import java.util.function.LongFunction; + +public class LongRangeForecastsTest { + + @Test + void longRangeForecast() { + // pure weather implementation: every holiday is sunny + LongFunction alwaysSunny = daysForward -> true; + var actual = new LongRangeForecasts().longRangeForecast(alwaysSunny); + Approvals.verify(printIceCreamForecast(actual)); + } + + private static String printIceCreamForecast(Map actual) { + var printedResult = new StringBuilder(); + printedResult.append("{"); + var sortedKeys = new ArrayList<>(actual.keySet()); + Collections.sort(sortedKeys); + for (IceCream key : sortedKeys) { + printedResult.append(key); + printedResult.append(":"); + printedResult.append(actual.get(key)); + printedResult.append(", "); + } + printedResult.append("}"); + return printedResult.toString(); + } +} +``` + +Approved output: + +``` +{Strawberry:12, Chocolate:12, Vanilla:17, } +``` + +**Checkpoint:** `mvn test` is green — deterministically. Nothing in the test path +touches `Scorer`. *What just became testable that was not before?* + +--- + +## Debrief + +### The dead call the lambda exposed + +![](svg/05-debrief-dead-call.svg) + +`scorer.updateSelection()` re-rolls `Scorer.flavour` — state that nothing in this +method ever reads (its only real consumer is the unrelated `salesForecast()`; it is +copy-paste residue in the long-range loop). Delete it from the lambda: the approved +file does not change. The refactoring didn't just hide the dependency — it exposed +that one of the two calls was dead all along. + +### Anti-pattern — the drift: scorer left behind in the extracted method + +![](svg/06-anti-pattern-drift.svg) + +If you lambda-ize only the value-producing call and leave `updateSelection()` as +"loop machinery", the extraction carries the `scorer` field into the new method: + +```java +// DRIFTED — the method still depends on the field; step 4 (move it +// to another class) would be blocked, and the split is cosmetic. +private List collectExpectedWeather( + LongFunction dayOffsetToWeather) { + var expectedWeather = new ArrayList(); + for (Instant interestingDate : DATES) { + scorer.updateSelection(); // ← LEFT BEHIND + long daysForward = ChronoUnit.DAYS.between(ANCHOR_DATE, interestingDate); + expectedWeather.add(dayOffsetToWeather.apply(daysForward)); + } + return expectedWeather; +} +``` + +Worse: the test still passes (the stray call only mutates state nothing reads), so +**green ≠ drift-free**. Catch it mechanically: `grep scorer` on the extracted method +must return nothing, or try to make it `static` — it won't compile with a field +reference. + +--- + +## Next moves (not storyboarded yet) + +These are steps 4 and 6 of the full pattern — the learning hour stops before them: + +- **Step 4:** move `collectExpectedWeather` to a class where `Scorer` is unknown. + (It is already `static` and scorer-free — the move is now unblocked.) +- **Step 6:** more pure-lambda combinations are now one line each: + `daysForward -> false` (no sunny holidays → all flavours 10), + `daysForward -> daysForward == 5` (exactly one sunny holiday, not at index 1 → all 10). +- The design view: the lambda is the smallest **Port** (function signature) + + **Adapter** (the extracted code with the unwanted dependency) that could work — + see the Role Interface discussion in the pattern draft. diff --git a/ideas/split-loop-with-lambda/storyboard/svg/00-pristine.svg b/ideas/split-loop-with-lambda/storyboard/svg/00-pristine.svg new file mode 100644 index 0000000..1fecba2 --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/00-pristine.svg @@ -0,0 +1,47 @@ + + + + + + + + Step 0 — one method, three loops, one unwanted dependency + The weather loop reaches outside the method to Scorer (which uses Random) — that is why the test is red & flaky. + + + + longRangeForecast() + + + + for (IceCream flavour : IceCream.values()) + result.put(flavour, 0); + + + + for (Instant interestingDate : DATES) + scorer.updateSelection(); + daysForward = DAYS.between(ANCHOR, date) + weather = scorer.lookupWeather(daysForward); + + + + for (flavour : keySet) result += 10 + if (sunnyHolidays > 2 && Vanilla) += 5 + if (sunnyHolidays > 1 && weather[1]) += 2 + scoring loop + + + + Scorer + stands for the weather + service — uses Random + + + + + + + approval test + red & flaky ✗ + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/01-extract-lambda.svg b/ideas/split-loop-with-lambda/storyboard/svg/01-extract-lambda.svg new file mode 100644 index 0000000..682b7ca --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/01-extract-lambda.svg @@ -0,0 +1,48 @@ + + + + + + + + Step 1 — extract the dependency-using code into a lambda + Both scorer calls go inside the lambda — the void one too. The invocation stays in the loop. + + + + longRangeForecast() + + + + for (IceCream flavour : IceCream.values()) + result.put(flavour, 0); + + + + for (Instant interestingDate : DATES) + + + + λ dayOffsetToWeather + scorer.updateSelection(); + return scorer.lookupWeather(days); + + + + expectedWeather.add(λ.apply(daysForward)); + + + + for (flavour : keySet) result += 10 + +5 / +2 ifs, unchanged + scoring loop + + + + Scorer + still called — + but only via the λ now + + + + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/02-slide-lambda.svg b/ideas/split-loop-with-lambda/storyboard/svg/02-slide-lambda.svg new file mode 100644 index 0000000..cc788f7 --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/02-slide-lambda.svg @@ -0,0 +1,56 @@ + + + + + + + + + + + Step 2 — slide the lambda definition to the top of the method + Slide, don't move: the definition slides up, the invocation stays exactly where it was. + + + + longRangeForecast() + + + + λ dayOffsetToWeather + scorer.updateSelection(); + return scorer.lookupWeather(days); + + + + for (IceCream flavour : IceCream.values()) + result.put(flavour, 0); + + + + for (Instant interestingDate : DATES) + + + + λ used to be defined here + + + expectedWeather.add(λ.apply(daysForward)); + + + + scoring loop: += 10 / +5 / +2 + + + + slide + + + + Scorer + still called — + but only via the λ now + + + + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/03-extract-method.svg b/ideas/split-loop-with-lambda/storyboard/svg/03-extract-method.svg new file mode 100644 index 0000000..edc5661 --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/03-extract-method.svg @@ -0,0 +1,67 @@ + + + + + + + + + + + Step 3 — extract the weather loop; the lambda becomes a parameter + Extract Method on the expectedWeather block. Checkpoint: grep scorer → no hits; try static → compiles. + + + + longRangeForecast() + + + + λ dayOffsetToWeather + scorer.updateSelection(); + return scorer.lookupWeather(daysForward); + + + + reset loop: result.put(flavour, 0) + + + + expectedWeather = + collectExpectedWeather(λ) + + + + for (flavour : keySet) result += 10 + if (sunny > 2 && Vanilla) += 5 + if (sunny > 1 && weather[1]) += 2 + + + + λ becomes a parameter + + + + collectExpectedWeather + static — no fields, all inputs are parameters + + + + λ LongFunction<Boolean> dayOffsetToWeather + + + + for (Instant d : DATES) + daysForward = between(ANCHOR, d) + expected.add(λ.apply(daysForward)) + no scorer reference in here + + return expectedWeather; + + + + Scorer + out of reach from in here + + ✗ cannot reach in + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/03b-overload.svg b/ideas/split-loop-with-lambda/storyboard/svg/03b-overload.svg new file mode 100644 index 0000000..bd06e8f --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/03b-overload.svg @@ -0,0 +1,53 @@ + + + + + + + + + + + Inset — the overload: let the λ come from outside + Add the λ as a parameter (Change Signature), drop the local. A no-arg overload bakes in the real λ — callers unchanged. + + + + longRangeForecast() + no-arg overload — existing callers keep working + + + daysForward -> { + scorer.updateSelection(); + return scorer.lookupWeather(days); + } + + return longRangeForecast(λ); + the "real" weather lambda lives here + + + + delegates + + + + longRangeForecast(λ) + the seam: weather is a parameter now + + + expectedWeather = + collectExpectedWeather(λ) + + + reset loop: result.put(flavour, 0) + + + scoring loop: += 10 / +5 / +2 + return result; + + + + Scorer + only the no-arg overload knows it + + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/04-pure-test.svg b/ideas/split-loop-with-lambda/storyboard/svg/04-pure-test.svg new file mode 100644 index 0000000..a6c96d5 --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/04-pure-test.svg @@ -0,0 +1,51 @@ + + + + + + + + + + + Step 5-lite — test through the seam with a pure lambda + The test passes alwaysSunny. Scorer is never touched: no Random, deterministic, glanceable. + + + + LongRangeForecastsTest + LongFunction<Boolean> + alwaysSunny = days -> true + longRangeForecast(alwaysSunny) + + + + longRangeForecast(λ) + collectExpected- + Weather(λ) + reset loop + scoring loop + += 10 / +5 / +2 + + + + collectExpectedWeather(λ) + for d : DATES + expected.add(λ.apply(d)) + return expectedWeather + pure ✓ + + + + alwaysSunny + + + + + Scorer + never called by the test ✗ + + ✗ + + Approved: {Strawberry:12, Chocolate:12, Vanilla:17, } — every run, not just a lucky roll. + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/05-debrief-dead-call.svg b/ideas/split-loop-with-lambda/storyboard/svg/05-debrief-dead-call.svg new file mode 100644 index 0000000..78b4791 --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/05-debrief-dead-call.svg @@ -0,0 +1,24 @@ + + + Debrief — the lambda made dead code visible + updateSelection() re-rolls Scorer.flavour — state nothing in this method ever reads. Delete it; the approved file is unchanged. + + + + daysForward -> { + scorer.updateSelection(); + + ✗ delete me + return scorer.lookupWeather(daysForward); + } + + + ✓ approved file unchanged: + {Strawberry:12, Chocolate:12, Vanilla:17, } + + + + Why: Scorer.flavour's only real consumer is the unrelated + salesForecast(). In the long-range loop it was copy-paste + residue — visible only once the lambda made it explicit. + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/06-anti-pattern-drift.svg b/ideas/split-loop-with-lambda/storyboard/svg/06-anti-pattern-drift.svg new file mode 100644 index 0000000..0aa323d --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/06-anti-pattern-drift.svg @@ -0,0 +1,40 @@ + + + + + + + + Anti-pattern — the drift: scorer left behind in the extracted method + Lambda-ize only the value-producing call, and the dependency travels with the extraction. + Happened in 2 of 6 agent rehearsal runs — behaviour passed, design goal silently failed. + + + + collectExpectedWeather(λ) + ✗ + + + for (Instant d : DATES) + scorer.updateSelection(); ✗ + daysForward = between(ANCHOR, d) + expected.add(λ.apply(daysForward)) + + not static anymore — it uses the field + + + + Scorer + still wired in + + + + Behaviour looks fine (the test can still pass) — but the split is cosmetic: + step 4, moving the method to a class without Scorer, is now blocked. + + + + Catch it mechanically: + • grep scorer on the extracted method → no hits + • try static → won't compile with a field ref + diff --git a/ideas/split-loop-with-lambda/storyboard/svg/legend.svg b/ideas/split-loop-with-lambda/storyboard/svg/legend.svg new file mode 100644 index 0000000..aad99fd --- /dev/null +++ b/ideas/split-loop-with-lambda/storyboard/svg/legend.svg @@ -0,0 +1,41 @@ + + + + + + + + + + + + + + How to read the diagrams + + + composed method / class under test + + + loop — the thing we split / extract + + + unwanted dependency — Scorer ≈ weather service + + + dependency call — the thing that wants to disappear + + + λ — a function value + + + pure / test + + + slide — definition moves, invocation stays + + + extraction / other motion + + Code lines inside boxes are monospace. Red = the unwanted dependency; purple = the lambda and everything it makes possible. +