Fix critical bugs and add tests for all untested JS sibling files - #283
Merged
Merged
Conversation
- hash-table-linear-probing.js: fix hash function that always returned
NaN (reduce callback returned the function reference instead of
invoking it), causing an infinite loop when putting 2+ distinct keys.
Also fixed the same bug in the unused #djb2HashCode method, and fixed
#verifyRemoveSideEffect to wrap its scan index so colliding keys that
wrapped around the table aren't lost after a remove().
- hash-table-separate-chaining.js: fix get() where the callback's
return inside forEach didn't propagate, so it always returned
undefined. Fix toString() which JSON.stringify'd the LinkedList
instance (yielding '{}') for every bucket after the first instead of
calling its toString().
- Rewrote hash-table-collision.test.ts to test correct behavior instead
of documenting the bugs, and added collision/wraparound coverage.
- Added new test files for previously under-tested JS sibling modules:
linked-list_.js, deque.js, comparator.js.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While reviewing test coverage, discovered that nearly all of the hand-written plain-JS "sibling" implementations (parallel to the tested TypeScript versions) had zero test coverage — they were never
require()'d by any test, so bugs could hide indefinitely. This PR adds dedicated tests for every such file and fixes every real bug found along the way.Overall: 677 tests passing, statement coverage 94.3% → 98.65%.
Bugs fixed
src/08-dictionary-hash/hash-table-linear-probing.js: hash function always returnedNaN(areducecallback returned a function reference instead of invoking it), causing an infinite loop when storing 2+ distinct keys. Also fixed#verifyRemoveSideEffect's scan index not wrapping around the table, which could silently lose a colliding key afterremove().hash-table-separate-chaining.js:get()'sreturninsideforEachnever propagated, so it always returnedundefined.toString()calledJSON.stringify()on the internalLinkedListinstance for all but the first bucket, producing"{}"instead of the list's contents.hash-table.js:put()used&&instead of||for its null-check (allowed a null key through).remove()used a truthy check that could never remove falsy values (0,'',false).toString()crashed on an empty table and mis-formatted objects in the first bucket.src/10-tree/fenwick-tree.js/segment-tree.js: missingmodule.exports— these classes could never berequire()'d at all.binary-search-tree.js,avl-tree.js,red-black-tree.js: internalrequire()s lacked explicit.jsextensions, resolving to the.tssiblings instead and throwing at construction time.avl-tree.js&red-black-tree.js: both subclasses declared their own private#root, shadowing the base class's#rootused by inheritedsearch/min/max/traversal methods. These were silently broken —searchalways returnedfalse,min/maxalwaysnull, traversals were no-ops — for both classes, the whole time.red-black-tree.js: rotation helpers never returned the new subtree root, corrupting the tree on removal-triggered rebalancing. Also fixed a staleparentpointer left after promoting a single child during removal.src/07-set/set.jshas()/getSizeWithoutSizeProperty()calledthis.#items.hasOwnProperty()directly — throws if a value like"hasOwnProperty"was ever stored, since it shadows the prototype method.src/06-linked-list/circular-linked-list_.jsprepend()mutated a stray publicheadproperty instead of the private#head, silently desyncing the list.removeAt()called a nonexistent#removeFromMiddlemethod — a hard crash for any middle-position removal.remove()/indexOf()contained leftover TypeScript type annotations, which are invalid syntax in a plain.jsfile and broke module parsing entirely.queue.js,stack.js,doubly-linked-list_.jstoString()calleditem.toString()on primitives, throwing onundefineditems. Switched toString(item)to match the.tssiblings.src/13-graph/*.js,src/12-trie/trie.js: no.js-specific bugs found — added full test coverage (100% stmts/branch) as a safety net.Known pre-existing issues found but intentionally left unfixed
(identical in both
.jsand.ts, so out of scope for this.js-only pass)kruskal.js/.ts:find()uses a falsy parent-pointer check that mistreats vertex0as "no parent," which can leave a vertex disconnected from the MST result.segment-tree.js/.ts: the "no overlap" query base case always returns0, which is only a valid identity forsum, notmin/max.red-black-tree.js/.ts: rare duplicate-value delete sequences can produce a structurally invalid BST (fuzz-tested 300+ trials with distinct values — no failures).Tests added
New
__test__files for every previously-untested.jsmodule:hash-table-js,set-js,trie-js,avl-tree-js,binary-search-tree-js,red-black-tree-js,fenwick-tree-js,segment-tree-js,graph-js,bfs-js,dfs-js,dijkstra-js,floyd-warshall-js,kruskal-js,prim-js,queue-js,stack-js,doubly-linked-list_,circular-linked-list_, plus the earlierlinked-list_,deque-js, andcomparatortests and rewrittenhash-table-collisiontests.