goal
Do not just ask “does it run?” Ask whether it satisfies the requirements.
A good code reader looks for requirement fit, hidden regression risk, clear boundaries, appropriate abstractions, useful tests, staging evidence, and basic code quality.
weekly read code lab #1
Best Score Review: learn to read code before you approve it.
Writing code is only half the skill. This section is for practicing code review: understand the task, inspect the diff, question the logic, and decide whether the change is safe to ship.
How this exercise works
Read the task, scan the diff, then scroll through each review question. The related diff lines highlight as you move through the page. Your notes are saved locally in your browser only.
original task
Save the best score after a completed game.
Requested change
When a player finishes Magnetic Blob Tumor Run, save the score to localStorage only if the player wins and the score is higher than the existing best score. Show the saved best score on the next visit.
Existing requirements that must still hold
- Losing a run must not overwrite the best score.
- A corrupted localStorage value must not break the page.
- The game should still reset cleanly after each run.
- The UI should show a clear mission summary after the run ends.
diff view
Practice review diff
This is intentionally imperfect. Your job is to read it like a reviewer, not to assume the green lines are correct.
diff --git a/magnetic-blob-tumor-run/game-results.js b/magnetic-blob-tumor-run/game-results.js
@@ -1,18 +1,53 @@
1 const BEST_SCORE_KEY = "magneticBlobBestScore";
2 const LAST_RUN_KEY = "magneticBlobLastRun";
3
4-export function finishRun(state) {
5- const won = state.tumorHealth <= 0;
6- return { won, score: state.score };
7-}
8+export function finishRun(state) {
9+ const summary = buildRunSummary(state);
10+ persistRunSummary(summary);
11+ resetTemporaryRunState(state);
12+ return summary;
13+}
14
15+function buildRunSummary(state) {
16+ return {
17+ won: state.tumorHealth <= 0,
18+ score: state.score,
19+ medicineLeft: state.medicine,
20+ durationMs: Date.now() - state.startedAt
21+ };
22+}
23
24+function persistRunSummary(summary) {
25+ const previousBest = Number(localStorage.getItem(BEST_SCORE_KEY));
26+ localStorage.setItem(LAST_RUN_KEY, JSON.stringify(summary));
27+
28+ if (summary.score >= previousBest || summary.won) {
29+ localStorage.setItem(BEST_SCORE_KEY, summary.score);
30+ }
31+}
32
33+function resetTemporaryRunState(state) {
34+ state.medicine = 0;
35+ state.tumorHealth = 100;
36+ state.startedAt = Date.now();
37+}
38
39+export function readBestScore() {
40+ return Number(localStorage.getItem(BEST_SCORE_KEY));
41+}question 1
What is the original task, and does this code implement a reasonable approach?
The task is not merely “save a score.” It is “save the best score only after a winning run, and only when the new score beats the old score.”
Hint
Line 28 is the main logic to challenge. Does summary.score >= previousBest || summary.won match “won and higher than best”?
question 2
Are previous understood requirements still satisfied?
Look for hidden breakage: losing runs, corrupted localStorage values, reset behavior, and whether the best score display can become NaN.
Hint
A losing run with a high score can overwrite the best score because of the ||. A corrupted stored value can make previousBest become NaN.
question 3
Are the abstractions appropriate, or did a modified function become too busy?
This diff splits summary creation, persistence, reset, and reading into separate functions. That is a good direction, but the persistence function owns business rules and browser storage at the same time.
Hint
A cleaner split would isolate the rule: shouldSaveBestScore(summary, previousBest). That rule is easier to test than localStorage-heavy code.
question 4
Is the business logic appropriately tested from a unit test perspective?
The important cases are small and testable: win + higher score, win + lower score, lose + higher score, lose + lower score, no existing score, and corrupted existing score.
Hint
If no tests cover losing runs and corrupted storage, this change is risky even if it works in one manual happy path.
question 5
Did the author provide reasonable staging or pseudo-production test evidence?
For a browser game, reasonable evidence could be a short test note: fresh browser, existing best score, losing run, winning run, mobile controls, and reload behavior.
Hint
Ask for proof that the best score displays correctly after refresh and that losing cannot overwrite it.
question 6
Is the code actually good: error-free, logical, readable, and maintainable?
The intent is readable, but the logic is wrong. There is also no storage validation, no localStorage error handling, and reset behavior mutates the incoming state inside a finish function.
Hint
Readable code can still be wrong. A reviewer should separate “I understand this” from “this is correct.”
final question
What would you write as the review verdict?
A strong review is specific and constructive. It should name the requirement mismatch, request tests, and suggest a smaller isolated business-rule function.
Example review comment
Request changes. The best-score condition appears to use ||, but the requirement says the run must be a win and the score must beat the previous best. Please add unit tests for losing high-score runs, winning lower-score runs, missing storage, and corrupted storage. I would also prefer extracting shouldSaveBestScore so the rule can be tested without localStorage.