From ed5db20c10a22a9da5e8f8b79c25ab022937067f Mon Sep 17 00:00:00 2001 From: k ho k ho? <6939499+kho-kho-kho@users.noreply.github.com> Date: Sun, 14 Aug 2022 18:14:03 -0700 Subject: [PATCH 1/5] Bugfix AVLTree comparator The original insertBalance function was doing raw value comparisons as opposed to using the tree's comparator. This is clearly unintentional, and would (ultimately) cause the structure to segfault when constructed with the stringData included in the updated test. I've added the fix, scanned the rest of the code for similar issues, and added the appropriate test case which passes successfully with the fix. The jest code coverage increases slightly as well with the changes. --- Data-Structures/Tree/AVLTree.js | 10 +++++----- Data-Structures/Tree/test/AVLTree.test.js | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/Data-Structures/Tree/AVLTree.js b/Data-Structures/Tree/AVLTree.js index d2b34362c0..1390defd35 100644 --- a/Data-Structures/Tree/AVLTree.js +++ b/Data-Structures/Tree/AVLTree.js @@ -90,14 +90,14 @@ const AVLTree = (function () { } // check if tree is balanced else balance it for insertion - const insertBalance = function (node, _val, balanceFactor) { - if (balanceFactor > 1 && _val < node._left._val) { + const insertBalance = function (node, _val, balanceFactor, tree) { + if (balanceFactor > 1 && tree._comp(_val, node._left._val) < 0) { return rightRotate(node) // Left Left Case } - if (balanceFactor < 1 && _val > node._right._val) { + if (balanceFactor < 1 && tree._comp(_val, node._right._val) > 0) { return leftRotate(node) // Right Right Case } - if (balanceFactor > 1 && _val > node._left._val) { + if (balanceFactor > 1 && tree._comp(_val, node._left._val) > 0) { node._left = leftRotate(node._left) // Left Right Case return rightRotate(node) } @@ -140,7 +140,7 @@ const AVLTree = (function () { } updateHeight(root) const balanceFactor = getHeightDifference(root) - return isValidBalanceFactor(balanceFactor) ? root : insertBalance(root, val, balanceFactor) + return isValidBalanceFactor(balanceFactor) ? root : insertBalance(root, val, balanceFactor, tree) } // delete am element diff --git a/Data-Structures/Tree/test/AVLTree.test.js b/Data-Structures/Tree/test/AVLTree.test.js index 64cdd3ff48..f7ebc455e5 100644 --- a/Data-Structures/Tree/test/AVLTree.test.js +++ b/Data-Structures/Tree/test/AVLTree.test.js @@ -5,26 +5,38 @@ describe('AVLTree Implementation: ', () => { const dataList = [] const demoData = [1, 4, 6, 22, 7, 99, 4, 66, 77, 98] + const avlStringTree = new AVLTree() + const collator = new Intl.Collator() + const stringData = ['S', 'W', 'z', 'B', 'a'] + beforeAll(() => { demoData.forEach(item => { if (avlTree.add(item)) { dataList.push(item) } }) + + avlStringTree._comp = collator.compare + stringData.forEach(item => avlStringTree.add(item)) }) it('checks if element is inserted properly', () => { expect(dataList.length).toEqual(avlTree.size) + expect(stringData.length).toEqual(avlStringTree.size) }) it('search if inserted element is present', () => { demoData.forEach(data => { expect(avlTree.find(data)).toBeTruthy() }) + stringData.forEach(data => { + expect(avlStringTree.find(data)).toBeTruthy() + }) }) it('deletes the inserted element', () => { const deleteElement = dataList[3] expect(avlTree.remove(deleteElement)).toBeTruthy() + expect(avlStringTree.remove(stringData[3])).toBeTruthy() }) }) From 2ea10b0a4f3436320157387a4a2b6224ddce8e29 Mon Sep 17 00:00:00 2001 From: k ho k ho? <6939499+kho-kho-kho@users.noreply.github.com> Date: Mon, 12 Sep 2022 20:28:02 -0700 Subject: [PATCH 2/5] 100% jest code coverage Added a couple of extra elements to the original test tree, and then removed elements in an order such that all previously uncovered branches of code are now covered. Also added an emptyTree structure to test some additional (trivial) base cases. --- Data-Structures/Tree/test/AVLTree.test.js | 40 ++++++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/Data-Structures/Tree/test/AVLTree.test.js b/Data-Structures/Tree/test/AVLTree.test.js index f7ebc455e5..9d4c25f9ff 100644 --- a/Data-Structures/Tree/test/AVLTree.test.js +++ b/Data-Structures/Tree/test/AVLTree.test.js @@ -3,12 +3,14 @@ import { AVLTree } from '../AVLTree' describe('AVLTree Implementation: ', () => { const avlTree = new AVLTree() const dataList = [] - const demoData = [1, 4, 6, 22, 7, 99, 4, 66, 77, 98] + const demoData = [1, 4, 6, 22, 7, 99, 4, 66, 77, 98, 87, 54, 32, 15] const avlStringTree = new AVLTree() const collator = new Intl.Collator() const stringData = ['S', 'W', 'z', 'B', 'a'] + const emptyTree = new AVLTree(collator.compare) + beforeAll(() => { demoData.forEach(item => { if (avlTree.add(item)) { @@ -20,6 +22,11 @@ describe('AVLTree Implementation: ', () => { stringData.forEach(item => avlStringTree.add(item)) }) + it('delete and search from empty tree', () => { + expect(emptyTree.remove(0)).toBeFalsy() + expect(emptyTree.find(0)).toBeFalsy() + }) + it('checks if element is inserted properly', () => { expect(dataList.length).toEqual(avlTree.size) expect(stringData.length).toEqual(avlStringTree.size) @@ -34,9 +41,32 @@ describe('AVLTree Implementation: ', () => { }) }) - it('deletes the inserted element', () => { - const deleteElement = dataList[3] - expect(avlTree.remove(deleteElement)).toBeTruthy() - expect(avlStringTree.remove(stringData[3])).toBeTruthy() + it('delete element with two valid children', () => { + expect(avlTree.remove(77)).toBeTruthy() + }) + + it('delete element missing L-child', () => { + expect(avlTree.remove(98)).toBeTruthy() + }) + + it('delete elements forcing single R-rotation', () => { + expect(avlTree.remove(99)).toBeTruthy() + expect(avlTree.remove(87)).toBeTruthy() }) + + it('delete elements forcing R-rotation and L-rotation', () => { + expect(avlTree.remove(1)).toBeTruthy() + expect(avlTree.remove(4)).toBeTruthy() + }) + + it('delete elements forcing single L-rotation', () => { + expect(avlTree.remove(7)).toBeTruthy() + expect(avlTree.remove(15)).toBeTruthy() + expect(avlTree.remove(6)).toBeTruthy() + }) + + it('delete element forcing single L-rotation and R-rotation', () => { + expect(avlTree.remove(66)).toBeTruthy() + }) + }) From 5f3b57c6c7e909917882e20ddf35d185dab2270f Mon Sep 17 00:00:00 2001 From: k ho k ho? <6939499+kho-kho-kho@users.noreply.github.com> Date: Mon, 12 Sep 2022 22:16:18 -0700 Subject: [PATCH 3/5] standard style fix missed this from my previous commit --- Data-Structures/Tree/test/AVLTree.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/Data-Structures/Tree/test/AVLTree.test.js b/Data-Structures/Tree/test/AVLTree.test.js index 9d4c25f9ff..0b2f485174 100644 --- a/Data-Structures/Tree/test/AVLTree.test.js +++ b/Data-Structures/Tree/test/AVLTree.test.js @@ -68,5 +68,4 @@ describe('AVLTree Implementation: ', () => { it('delete element forcing single L-rotation and R-rotation', () => { expect(avlTree.remove(66)).toBeTruthy() }) - }) From 1ab0a8299cefe1e168dadca309c4663caabcc0fb Mon Sep 17 00:00:00 2001 From: k ho k ho? <6939499+kho-kho-kho@users.noreply.github.com> Date: Fri, 16 Sep 2022 18:06:29 -0700 Subject: [PATCH 4/5] Iterative & in-place BFS An iterative analog to the traditional recursive breadth-first-search algorithm for binary trees. This in-place solution uses the pre-existing "traversal" array for both tracking the algorithm as well as storing the result. Also tweaked old code by resetting the traversal array each time the tree is traversed (otherwise you're only allowed to traverse a tree once which doesn't seem correct even with a single traversal function). --- Trees/BreadthFirstTreeTraversal.js | 22 +++++++++++++++++++- Trees/test/BreadthFirstTreeTraversal.test.js | 9 ++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Trees/BreadthFirstTreeTraversal.js b/Trees/BreadthFirstTreeTraversal.js index 0b4520612b..c93d3066f3 100644 --- a/Trees/BreadthFirstTreeTraversal.js +++ b/Trees/BreadthFirstTreeTraversal.js @@ -17,7 +17,27 @@ class BinaryTree { this.traversal = [] } - breadthFirst () { + breadthFirstIterative () { + this.traversal = [] + if (this.root) { + this.traversal.push(this.root) + } + let currentSize = this.traversal.length + for (let i = 0; i < currentSize; i++) { + if (this.traversal[i].left) { + this.traversal.push(this.traversal[i].left) + } + if (this.traversal[i].right) { + this.traversal.push(this.traversal[i].right) + } + this.traversal[i] = this.traversal[i].data + currentSize = this.traversal.length + } + return this.traversal + } + + breadthFirstRecursive () { + this.traversal = [] const h = this.getHeight(this.root) for (let i = 0; i !== h; i++) { this.traverseLevel(this.root, i) diff --git a/Trees/test/BreadthFirstTreeTraversal.test.js b/Trees/test/BreadthFirstTreeTraversal.test.js index 8e325e02d4..a6b795f31e 100644 --- a/Trees/test/BreadthFirstTreeTraversal.test.js +++ b/Trees/test/BreadthFirstTreeTraversal.test.js @@ -19,9 +19,14 @@ describe('Breadth First Tree Traversal', () => { // / \ \ // 3 6 9 - it('Binary tree - Level order traversal', () => { + it('Binary tree - Level order recursive traversal', () => { expect(binaryTree.traversal).toStrictEqual([]) - const traversal = binaryTree.breadthFirst() + const traversal = binaryTree.breadthFirstRecursive() + expect(traversal).toStrictEqual([7, 5, 8, 3, 6, 9]) + }) + + it('Binary tree - Level order iterative traversal', () => { + const traversal = binaryTree.breadthFirstIterative() expect(traversal).toStrictEqual([7, 5, 8, 3, 6, 9]) }) }) From e0d683e06e85013980f34ed1dde2a0a2c0c192fb Mon Sep 17 00:00:00 2001 From: k ho k ho? <6939499+kho-kho-kho@users.noreply.github.com> Date: Sun, 18 Sep 2022 16:52:40 -0700 Subject: [PATCH 5/5] Update BreadthFirstTreeTraversal.js got rid of unnecessary currentSize added currentNode for clarity --- Trees/BreadthFirstTreeTraversal.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Trees/BreadthFirstTreeTraversal.js b/Trees/BreadthFirstTreeTraversal.js index c93d3066f3..10fdc0aeb6 100644 --- a/Trees/BreadthFirstTreeTraversal.js +++ b/Trees/BreadthFirstTreeTraversal.js @@ -22,16 +22,15 @@ class BinaryTree { if (this.root) { this.traversal.push(this.root) } - let currentSize = this.traversal.length - for (let i = 0; i < currentSize; i++) { - if (this.traversal[i].left) { - this.traversal.push(this.traversal[i].left) + for (let i = 0; i < this.traversal.length; i++) { + const currentNode = this.traversal[i] + if (currentNode.left) { + this.traversal.push(currentNode.left) } - if (this.traversal[i].right) { - this.traversal.push(this.traversal[i].right) + if (currentNode.right) { + this.traversal.push(currentNode.right) } - this.traversal[i] = this.traversal[i].data - currentSize = this.traversal.length + this.traversal[i] = currentNode.data } return this.traversal }