Add storyboard handout for split-loop-with-lambda mechanics
Step-by-step refactoring instructions as a single markdown handout with one diagram per step (legend + steps 0-3, overload inset, pure test, debrief slides for the dead call and the drift anti-pattern). Derived from the pattern draft, the learning hour plan, and the rehearsal findings; slide-ready for later.
This commit is contained in:
Your Name
@@ -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
|
||||
|
||||
|
||||
|
||||
---
|
||||
|
||||
## 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).
|
||||
|
||||
|
||||
|
||||
**Before:**
|
||||
|
||||
```java
|
||||
public Map<IceCream, Integer> longRangeForecast() {
|
||||
var result = new HashMap<IceCream, Integer>();
|
||||
for (IceCream flavour : IceCream.values()) {
|
||||
result.put(flavour, 0);
|
||||
}
|
||||
List<Boolean> 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.
|
||||
|
||||
|
||||
|
||||
**After:**
|
||||
|
||||
```java
|
||||
public Map<IceCream, Integer> longRangeForecast() {
|
||||
var result = new HashMap<IceCream, Integer>();
|
||||
for (IceCream flavour : IceCream.values()) {
|
||||
result.put(flavour, 0);
|
||||
}
|
||||
List<Boolean> expectedWeather = new ArrayList<>();
|
||||
LongFunction<Boolean> 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.
|
||||
|
||||
|
||||
|
||||
**After:**
|
||||
|
||||
```java
|
||||
public Map<IceCream, Integer> longRangeForecast() {
|
||||
LongFunction<Boolean> dayOffsetToWeather = daysForward -> { // ← SLID to the top...
|
||||
scorer.updateSelection();
|
||||
return scorer.lookupWeather(daysForward);
|
||||
};
|
||||
var result = new HashMap<IceCream, Integer>();
|
||||
for (IceCream flavour : IceCream.values()) {
|
||||
result.put(flavour, 0);
|
||||
}
|
||||
List<Boolean> 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<Boolean>`) 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.
|
||||
|
||||
|
||||
|
||||
**Before:** end state of step 2 (above).
|
||||
|
||||
**After:**
|
||||
|
||||
```java
|
||||
public Map<IceCream, Integer> longRangeForecast() {
|
||||
LongFunction<Boolean> dayOffsetToWeather = daysForward -> {
|
||||
scorer.updateSelection();
|
||||
return scorer.lookupWeather(daysForward);
|
||||
};
|
||||
var result = new HashMap<IceCream, Integer>();
|
||||
for (IceCream flavour : IceCream.values()) {
|
||||
result.put(flavour, 0);
|
||||
}
|
||||
List<Boolean> 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<Boolean> collectExpectedWeather( // ← NEW — and already static
|
||||
LongFunction<Boolean> dayOffsetToWeather) {
|
||||
var expectedWeather = new ArrayList<Boolean>();
|
||||
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.
|
||||
|
||||
|
||||
|
||||
**After:**
|
||||
|
||||
```java
|
||||
public Map<IceCream, Integer> longRangeForecast() { // ← no-arg overload: callers unchanged
|
||||
return longRangeForecast(daysForward -> {
|
||||
scorer.updateSelection();
|
||||
return scorer.lookupWeather(daysForward);
|
||||
});
|
||||
}
|
||||
|
||||
public Map<IceCream, Integer> longRangeForecast( // ← the seam: λ is a parameter
|
||||
LongFunction<Boolean> 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.
|
||||
|
||||
|
||||
|
||||
**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<Boolean> alwaysSunny = daysForward -> true;
|
||||
var actual = new LongRangeForecasts().longRangeForecast(alwaysSunny);
|
||||
Approvals.verify(printIceCreamForecast(actual));
|
||||
}
|
||||
|
||||
private static String printIceCreamForecast(Map<IceCream, Integer> 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
|
||||
|
||||
|
||||
|
||||
`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
|
||||
|
||||
|
||||
|
||||
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<Boolean> collectExpectedWeather(
|
||||
LongFunction<Boolean> dayOffsetToWeather) {
|
||||
var expectedWeather = new ArrayList<Boolean>();
|
||||
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.
|
||||
Reference in New Issue
Block a user