JAVASCRIPT / PROJECTS AND PROFESSIONAL CRAFT
Reviewing your own code for style and mistakes
Review your own JavaScript before anyone else does, in separate passes that catch in-place mutation, unawaited promises and misspelled properties.
What you will learn
- Read the diff, not the whole file, in separate passes for correctness and clarity
- Catch in-place mutation: sort, reverse and splice rewrite the caller's array
- Check that every async function in the diff is awaited or returned by someone
- Read undefined in output as a misspelled property, not as missing data
Understanding Reviewing your own code for style and mistakes
The reason you miss bugs in your own code is that you do not read it, you recognise it. Your memory of what you meant fills in the missing await and silently corrects u.firstname to u.firstName, so the fix is to change the reading conditions: wait ten minutes, look at the changed lines rather than the file, and read them somewhere other than the editor you typed them in. A pull request view, git diff in a terminal, or reading the changed lines aloud all work for the same reason, which is that they stop you skimming a shape you already know.
Give each pass exactly one question. A correctness pass asks what each new function does with an empty array, a zero, a missing property and a promise that rejects, and you run those inputs instead of picturing them, because picturing them is what got you here. A clarity pass asks whether any name made you re-read the line, whether a function's name needs the word 'and', and whether commented-out code or a leftover console.log is still in the diff. Mixing the two passes means you spend your attention on quote marks and step straight over the off-by-one.
What deserves a fixed checklist in JavaScript is the set of ways the language fails quietly: methods that mutate in place (sort, reverse, splice, push on an array a caller still holds), an async function nobody awaits, == comparing across types, sort with no comparator ordering numbers as strings, a catch block that logs and continues. Keep your own version of that list, built from bugs you actually shipped. Five items you genuinely repeat will catch more than fifty generic ones you skim.
None
// First draft: return the n highest scores.
function topScores(scores, n) {
return scores.sort((a, b) => b - a).slice(0, n);
}
// After the review pass: sort() reorders the array it was handed, so copy first.
function topScoresReviewed(scores, n) {
return [...scores].sort((a, b) => b - a).slice(0, n);
}
const raw = [7, 3, 10, 1];
console.log('top 2:', JSON.stringify(topScores(raw, 2)));
console.log('caller array now:', JSON.stringify(raw));
const rawAgain = [7, 3, 10, 1];
console.log('top 2:', JSON.stringify(topScoresReviewed(rawAgain, 2)));
console.log('caller array now:', JSON.stringify(rawAgain));Self-review works only when you change how you read the code, taking one narrow question per pass over the changed lines, because your memory of the intent hides what you actually wrote.
Worked examples
Boundary inputs before the happy path
The empty-array question is the cheapest correctness check to run on any new function that loops or reduces.
function average(nums) {
return nums.reduce((sum, n) => sum + n) / nums.length;
}
function averageReviewed(nums) {
if (nums.length === 0) return 0;
return nums.reduce((sum, n) => sum + n, 0) / nums.length;
}
console.log(average([2, 4, 6]));
try {
console.log(average([]));
} catch (err) {
console.log('threw', err.name);
}
console.log(averageReviewed([]));Example explained
Line 1reduce with no second argument seeds itself from element 0, so an empty array has nothing to start from and throws.
Line 2The happy-path call prints 4 and hides that entirely; only asking what the empty case does surfaces it.
Line 3averageReviewed answers the boundary out loud: length 0 returns 0 instead of throwing or yielding NaN.
Line 4err.name is printed rather than err.message because engines word that particular message differently.
Every async call needs an awaiter
An async callback handed to forEach is dropped on the floor, and the only symptom is a suspiciously empty result.
const delay = (ms, value) => new Promise((resolve) => setTimeout(resolve, ms, value));
async function loadWithForEach(ids) {
const out = [];
ids.forEach(async (id) => {
out.push(await delay(10, id));
});
return out;
}
async function loadWithForOf(ids) {
const out = [];
for (const id of ids) {
out.push(await delay(10, id));
}
return out;
}
async function main() {
console.log('forEach:', JSON.stringify(await loadWithForEach([1, 2, 3])));
console.log('for..of:', JSON.stringify(await loadWithForOf([1, 2, 3])));
}
main();Example explained
Line 1forEach discards the promise each async callback returns, so execution reaches return out while all three pushes are still pending.
Line 2Nothing throws and no syntax looks wrong, which is why the review question is 'who awaits this?' for every async in the diff.
Line 3for...of suspends the enclosing function at each await, so out is filled before the return runs.
undefined is a spelling report
Reading a misspelled property is legal JavaScript, so the mistake shows up as text in the UI instead of an error.
const user = { firstName: 'Ada', lastName: 'Lovelace' };
function label(u) {
return `${u.firstname} ${u.lastName}`;
}
console.log(label(user));
function required(obj, key) {
if (!Object.hasOwn(obj, key)) throw new Error(`missing key: ${key}`);
return obj[key];
}
try {
required(user, 'firstname');
} catch (err) {
console.log(err.message);
}Example explained
Line 1u.firstname is not an error: a missing property evaluates to undefined, and the template literal stringifies it without complaint.
Line 2So undefined anywhere in your output is a review signal about a name or an object shape, not about a missing value.
Line 3Object.hasOwn tests the property name itself, so the typo fails where it was made instead of leaking into a display string.
Line 4hasOwn is used rather than the in operator because in also reports inherited keys.
Important notes
A linter catches unused and shadowed bindings; only reading catches a plausible but wrong name, like userIds that actually holds user objects.
Keep renames in a separate commit from behaviour fixes, otherwise your next self-review faces a diff in which every line changed.
Common mistakes
Reading the whole file instead of the diff: attention is spent on code that has not changed and runs out before today's off-by-one.
Doing style and correctness in one pass: you tidy indentation and quote marks and ship the sort() that reorders the caller's array.
Reviewing immediately, in the same editor buffer, while your intent is fresh: you re-read the version in your head, so u.firstname stays invisible.
Try it yourself
Change, predict, then run
In a browser editor, write moveToFront(list, value) that returns the list with value first, then review it in two passes: pass one, call it with an empty list and with a value that is absent, and log the caller's array afterwards; pass two, rename anything you had to read twice and delete the logs you added.
Open the JavaScript workspaceCheck your understanding
Your feature works in the browser and both the formatter and the linter report no problems. Why is a manual read of the diff still likely to find a real bug?
- Passing lint means the logic has been checked, so the read can only improve formatting
- Linters cannot parse async functions, so those lines were never inspected at all
- Lint rules judge local patterns, so they cannot know that this sort() reorders an array the caller still reads
- A manual read is only needed for files the bundler leaves out of the build
Show answer
Whether [...scores].sort() was required depends on what the caller expects afterwards, which is not visible in the file being linted; the tool sees a valid call to a valid method. Option 0 is tempting because lint failures usually are real defects, but no-unused-vars passing says nothing about a missing await, a misspelled property, or a mutation that crosses a module boundary.