Run the code in these notes: a latent crash, a wrong complexity claim, and an empty file by alpersonalwebsite · Pull Request #2 · alpersonalwebsite/data-structures-and-algorithms

alpersonalwebsite · 2026-08-23T22:44:57Z

Notes on five data structures with runnable implementations. Every code block here parses, so a syntax
checker would have found nothing. The first commit builds one that runs the code instead, and the
rest of the branch is what it found.

A collision handler that broke on the second collision

03_0 / 03_1 walked the collision chain with const currentNode and reassigned it:

const currentNode = this.buckets[bucketIndex];
while (currentNode.next) { currentNode = currentNode.next; }

That throws TypeError: Assignment to constant variable. — but only once the loop body runs, which
needs a third key in one bucket. The example adds Casa, A, Adding: two in bucket 0, so the
first node's next is still null, the loop is skipped, and the demo passes.

Measured, adding a fourth key to that bucket:

with const (as written)   exit 1   TypeError: Assignment to constant variable.
with let    (fixed)       exit 0

get and getAll in the same file already used let, which is what makes this a slip rather than a
misunderstanding.

A complexity table that was wrong about its own queue

05_0 claimed Enqueue: arr[0] -> O(1) and "we are always adding/removing to/from the END". The
implementation enqueues with unshift, which inserts at index 0 and moves every element up one.

n=20000   unshift  18.6 ms    push  0.4 ms
n=40000   unshift  95.4 ms    push  1.6 ms
n=80000   unshift 400.9 ms    push  0.8 ms

Doubling n quadruples the total: a linear per-call cost. The note had reasoned about the wrong
operation — indexing an array is O(1), inserting at an index is not. In a repository about complexity,
about its own code.

Also checked and not a defect, recorded because I nearly filed it: get uses pop, which looks like
a stack in a file titled FIFO. But add uses unshift, so the first item added ends up at the end and
pop returns it. add(1), add(2), add(3) then get() returns 1. Correct FIFO.

Files that had drifted to nothing

file	was
`05_1_queue-and-stack.js`	0 bytes, against four implementations in its notes
`04_1_linked-list.js`	one method, against eight documented

A reader who cloned this and ran the queue file got nothing at all, silently. Both are now generated
from their notes by scripts/build-js.mjs, and npm test fails if they stop matching — the drift is
impossible rather than merely fixed. The notes are the source of truth: that is where the explanation
lives and what was maintained.

Two things the checker taught me about itself

Case-sensitive fence matching skipped a third of the code. The repo writes the tag three ways:
javascript (14), JavaScript (8), js (4). Matching lowercase only missed eight blocks including the
class definitions, and the result was ReferenceErrors that looked like defects in the notes rather than
a defect in my extractor.

Untagged fences are reported, not skipped. There are 76 correctly-bare fences here holding console
output, so the only way to tell code from output is to look inside. A checker that quietly ignores half
the code is worse than none, because its green result implies coverage it does not have.

The ## Result: blocks are deliberately not diffed. They are Firefox DevTools renderings
(Object { … }, <prototype>:), not node's, so comparing them would fail on formatting for every one
and the check would be off within a day.

Smaller findings

BinarSearchTree (20 occurrences, none correct) and HasthTable (13), renamed in code and in the
claimed output.
let HT twice and let Q three times in sections that extend the same object, so pasting a file top
to bottom gave SyntaxError: Identifier 'HT' has already been declared. The genuinely alternative
class Queue implementations are marked standalone instead.
The Greenkeeper badge is not broken, which is worse: it renders "Greenkeeper | Move to Snyk", so the
repo carried a live advert for a dead service. Removed.
package.json said license: ISC while LICENSE is MIT; repository, bugs and homepage used the
pre-rename name data-structures and worked only via GitHub's redirect; main and description were
empty; test was the failing default stub.
The timing wrappers printed Function took 1.8 milliseconds from all three files, dominated by module
load and console.log rather than the six insertions. Removed, and used as the closing example in the
complexity page of what Big O does not tell you.
00_runtime-complexity.md and 00_data-structures.md were both the literal string TO DO.... The
first is written (the description promises it); the second had no scope the README index does not cover.

Checks

Seven poison tests, each confirmed applied before its result was read: a syntax error, a runtime throw
(reported with the markdown block it came from), an untagged fence, a hand-edited .js, an edited .md
without a rebuild, the const bug plus a third key, and a tag reverted to uppercase which must still be
found. All seven behave.

One was defective on the first attempt and taught its own lesson: it had two parts and only one applied,
because the line it targeted has no trailing semicolon. diff said the file changed and I read that as
both parts landing. When a poison has two parts, assert both.

No dependencies added. npm test is node and git only.

Review round

Both count errors fixed and the CI note taken, in 3506a5c. CI green in 11s.

"76 correctly-bare fences" is 25 blocks. 76 is the number of lines equal to a bare fence, which
counts the closing fence of every tagged block too. Measured: 26 tagged blocks, 25 bare blocks, 76
bare-fence lines. That figure was the justification for the untagged-fence check, so being wrong about it
undermined the thing it argued for.

Fixed by making the checker print it rather than by correcting a sentence:

bare blocks holding output: 25, untagged fences containing code: 0

"One method against eight documented" is one against ten. Base 04_0 documents ten
LinkedList.prototype methods; base 04_1 implemented one; head implements all ten. Eight is what you
get excluding getHead/getTail as trivial accessors, which may have been the thought, but it is not
what the sentence said. Corrected in the README and in the generator that stamps the four .js headers.

The same wrong figures are in 6ffdb98 and e9cf3c7, which are pushed and cited in review, so they are
recorded here rather than amended.

That is the eighth and ninth wrong count in this effort. The repair I keep arriving at, and have now
applied here: derive a count where it carries information, delete it where it does not, and only write one
by hand when neither is possible.

CI added, and your argument for it is stronger here than in the sibling repos. The claim this PR makes
is that the .js files cannot drift from the notes, and the mechanism enforcing that was --check
inside a script someone has to remember to type. A conditional guarantee, and the condition is exactly
what let 05_1_queue-and-stack.js sit at zero bytes. No install step, since this repository has no
dependencies at all. Verified green from a fresh clone with no node_modules.

On your poison 7 method: checking the block count rather than the exit code was the right instinct and
better than what I did. Exit 0 cannot distinguish "found" from "silently skipped", and then making the
matcher case-sensitive to confirm the property is load-bearing is the step that actually proves it. That
is the same shape as a scheduled job that only ever skips looking identical to a broken one.

…rom them Two scripts, and the first one runs the code rather than parsing it, because parsing would have found nothing here. Every block in this repository parses. scripts/check-samples.mjs concatenates each notes file's code blocks in document order and executes them. That is the right unit: a file is one program spread across blocks, where one defines the class, the next adds a method to its prototype, and the next exercises it. Running the whole thing means a block has what earlier blocks defined, and an implementation that parses but does not work fails. The hash table is why. Its collision walk declared `const currentNode` and reassigned it in the loop, which parses perfectly and throws `TypeError: Assignment to constant variable.` the moment the loop body runs. Running the body needs a THIRD key in one bucket, and the example added two, so the demo passed. Only execution with three finds it. Two details in the extractor were found by it failing: CASE-INSENSITIVE FENCE TAGS. This repository writes the tag three ways, `javascript` (14), `JavaScript` (8) and `js` (4). Matching case-sensitively skipped eight blocks, including the ones defining the classes, and the result was ReferenceErrors that looked like defects in the notes rather than a defect in my extractor. Markdown does not care about the case; tooling has to be told. A `// check: standalone` marker starts a new program. 05_0 declares `class Queue` three times, once per approach to privacy, and concatenating alternatives is a redeclaration error rather than a defect in any of them. The marker lets a document say a block REPLACES what came before instead of extending it. UNTAGGED FENCES ARE REPORTED RATHER THAN SKIPPED. A code block with no language tag is invisible to tooling, and this repository has 76 correctly-bare fences holding console output, so the only way to tell them apart is to look inside. Anything untagged containing `class`, `prototype`, `function`, `=>` or `const` is flagged. A checker that quietly ignores half the code is worse than none, because its green result implies coverage it does not have. The `## Result:` blocks are NOT compared against actual output. They are Firefox DevTools renderings (`Object { … }`, `<prototype>:`), not node's, so a diff would fail on formatting for all of them and the check would be off within a day. What the run proves is that the code producing them executes. scripts/build-js.mjs generates each NN_1_*.js from its NN_0_*.md and `--check` fails if the committed file differs. The notes are the source of truth: that is where the explanation lives and what was maintained. Alternative implementations are wrapped in a block, since `class` and `let` are block-scoped and the three Queues can then share a name in one file. Seven poison tests, each confirmed applied before its result was read: a syntax error, a runtime throw (reported with the markdown block it came from), an untagged fence, a hand-edited .js, an edited .md without a rebuild, the const bug plus a third key, and a tag reverted to uppercase which must still be found. All seven behave. One of those poisons was defective on the first attempt and taught its own lesson: it had TWO parts, and only one applied, because the line it targeted has no trailing semicolon. `diff` said the file had changed and I read that as both parts landing. When a poison has two parts, assert both. package.json: the license said ISC while LICENSE is MIT; `repository`, `bugs` and `homepage` all used the pre-rename name `data-structures`, which works only through GitHub's redirect; `main` was empty and so was `description`; and `test` was the npm default failing stub. All fixed, and `test` now runs both checkers. No dependencies added.

THE HASH TABLE'S COLLISION WALK COULD NOT WALK. `add` declared `const currentNode` and reassigned it while following the chain, so the moment the loop body runs it throws: TypeError: Assignment to constant variable. Running the body needs a THIRD key in one bucket, because with two the first node's `next` is still null and the loop is skipped. The example adds `Casa`, `A` and `Adding`, which is two in bucket 0. Measured: with a fourth key in that bucket the pre-fix code exits 1 with the error above, and the fixed code exits 0. So a repository teaching collision handling had a collision handler that broke on the second collision, with a demo one key short of showing it. `get` and `getAll` in the same file already use `let`, which is what makes this a slip rather than a misunderstanding, and worth fixing quietly rather than rewriting the lesson. THE QUEUE'S COMPLEXITY TABLE WAS WRONG ABOUT THE QUEUE. It claimed "Enqueue: `arr[0]` -> O(1)" and that "we are always adding/removing to/from the END". The implementation enqueues with `unshift`, which inserts at index 0 and moves every element up one, and the note had reasoned about the wrong operation: indexing an array is O(1), inserting at an index is not. Measured, one run on node 24: n=20000 unshift 18.6 ms push 0.4 ms n=40000 unshift 95.4 ms push 1.6 ms n=80000 unshift 400.9 ms push 0.8 ms Doubling n quadruples the total, which is a linear per-call cost. The note now says O(n), shows the measurement, and explains why the original reasoning was about indexing rather than insertion. Also checked and NOT a defect, recorded because I nearly filed it: `get` uses `pop`, which looks like a stack in a file titled FIFO. But `add` uses `unshift`, so the first item added ends up at the end and `pop` returns it. Verified: add(1), add(2), add(3) then get() returns 1. The queue is correct FIFO. TWO MISSPELLED CLASS NAMES, in code and in the claimed output: `BinarSearchTree` (20 occurrences, none spelled correctly) and `HasthTable` (13). Renamed throughout, including inside the `## Result:` blocks so those still describe what the code prints. REDECLARATIONS THAT BREAK THE FILE IF YOU FOLLOW IT IN ORDER. `let HT` appeared twice in the hash table notes and `let Q` three times in the queue notes, in sections that extend the same object rather than replacing it, so pasting the file top to bottom gives `SyntaxError: Identifier 'HT' has already been declared`. The later ones now reassign. The three genuinely alternative `class Queue` implementations are marked `standalone` instead, since those really do replace each other. Fence tags normalised to one spelling, `javascript`, from a mix of three. One era note added rather than a rewrite: the private-fields section says class fields are "in stage 3 and moving through the ladder, so at future we should be able to" have private fields natively. They shipped. Verified `#data` works on node 24 and is genuinely private.

The runnable files and the notes had come apart, in one case completely: 04_1_linked-list.js one method (addHead) against eight documented in 04_0 05_1_queue-and-stack.js ZERO BYTES against four implementations in 05_0 So a reader who cloned the repository and ran the queue file got nothing at all, silently, and one who read the linked-list file got an eighth of the material. Both are now assembled from their notes by scripts/build-js.mjs, and `npm test` fails if they stop matching, which makes the drift impossible rather than merely fixed. The timing wrappers are gone. Each file printed `Function took 1.8 milliseconds` after its demo, and all three printed roughly the same figure because it was dominated by module load and `console.log` rather than by the six insertions being demonstrated. A wall-clock number on a six-element example says nothing about asymptotic behaviour, and printing one beside a Big O lesson misleads about the thing the repository is teaching. 05_1 wraps its three alternative Queue implementations in blocks, because `class` and `let` are block-scoped, so they share a name in one file exactly as they each stand alone in the notes.

00_runtime-complexity.md was the literal string `TO DO...`, nine bytes, while the repository description promises "with runtime complexity notes". It now states Big O against THESE implementations rather than the textbook ideal, which is the only version worth having: complexity is a property of how a structure is built, not of its name, and three of the ones here are slower than their labels suggest. the queue enqueues with `unshift`, so enqueue is O(n), measured the hash table reads only the first character, so same-letter keys chain and lookup is O(n) in k the tree never rebalances, so sorted input gives a right spine and every operation is O(n) The page closes on what Big O does not tell you, using this repository's own removed timing wrappers as the example: three structures all reporting 1.8 ms says nothing about growth. 00_data-structures.md was also `TO DO...` with no scope beyond its title, and the README index covers the overview it would have been, so it is deleted rather than invented. The README was 270 bytes: a title and two badges, one of them a Greenkeeper badge. That badge is not broken, which is worse. It renders "Greenkeeper | Move to Snyk", so the repository was carrying a live advertisement for a service that shut down, telling readers to migrate. Removed. The MIT badge stays and now agrees with both LICENSE and package.json, which it did not before. The README now says what the repository is, indexes the five notes with their runnable counterparts, documents the two checks, and records the three findings worth knowing rather than burying them in git history: the const that could not walk a chain, the complexity table that was wrong about its own queue, and the timers that measured `console.log`. .gitignore: added the `.env*` prefix glob and the `*.env` suffix, since neither implies the other and it had only the bare name plus four `.local` variants; a negation so a template stays committable; and `*.log`, where it previously listed only npm-debug and yarn-debug so any other log was tracked. Verified per pattern with git check-ignore.

coderabbitai · 2026-08-23T22:45:14Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)

review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: alpersonalwebsite/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 08f3be77-8693-4a28-81c5-7d3c4da8de22

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

_{Comment @coderabbitai help to get the list of available commands.}

…ecks in CI Two count errors from review, both in durable text, both understating. "76 CORRECTLY-BARE FENCES" IS 25 BLOCKS. 76 is the number of lines equal to a bare fence, which counts the closing fence of every tagged block as well. Measured: 26 tagged blocks, 25 bare blocks, 76 bare-fence lines. Since that figure is the justification for the untagged-fence check, being wrong about it undermines the thing it argues for. Fixed by making the checker PRINT it rather than by correcting a sentence: bare blocks holding output: 25, untagged fences containing code: 0 The number now cannot go stale and nobody has to trust prose about it. That is the general repair for this, and it is the one I keep arriving at: derive a count where it carries information, delete it where it does not, and only write it by hand when neither is possible. "ONE METHOD AGAINST EIGHT DOCUMENTED" IS ONE AGAINST TEN. Base 04_0 documents ten LinkedList.prototype methods and base 04_1 implemented one; head implements all ten. Eight is what you get excluding getHead and getTail as trivial accessors, which may have been the thought, but it is not what the sentence said. Corrected in the README and in the generator that stamps the .js headers, so all four generated files carry the right figure. That is the eighth and ninth wrong count in this effort, and both understate the work, which is the benign direction and not the point. CI ADDED, and the argument for it is stronger here than in the sibling repositories. The claim this pull request makes is that the .js files CANNOT drift from the notes, and the mechanism enforcing it was `--check` inside a script someone has to remember to type. That is a conditional guarantee, and the condition is exactly what let 05_1_queue-and-stack.js sit at zero bytes while its notes carried four implementations. A workflow makes it unconditional. No install step: this repository has no dependencies at all and both scripts use only what ships with node. Verified `npm test` green from a fresh clone with no node_modules.

alpersonalwebsite added 4 commits August 23, 2026 15:43

alpersonalwebsite merged commit 39a48ad into master Aug 23, 2026
2 checks passed

alpersonalwebsite deleted the improve-security-correctness-and-docs branch August 23, 2026 23:14

Navigation Menu

Run the code in these notes: a latent crash, a wrong complexity claim, and an empty file - #2

Run the code in these notes: a latent crash, a wrong complexity claim, and an empty file#2
alpersonalwebsite merged 5 commits into
masterfrom
improve-security-correctness-and-docs

alpersonalwebsite commented Aug 23, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 23, 2026 •

edited

Loading

Review skipped

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

alpersonalwebsite commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A collision handler that broke on the second collision

A complexity table that was wrong about its own queue

Files that had drifted to nothing

Two things the checker taught me about itself

Smaller findings

Checks

Review round

Uh oh!

coderabbitai Bot commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

alpersonalwebsite commented Aug 23, 2026 •

edited

Loading

coderabbitai Bot commented Aug 23, 2026 •

edited

Loading