Here's my implementation for a Binary Search Tree in JavaScript. All functions appear to be working properly except for the remove
function. Specifically, it seems to be removing nodes properly until there's 2 nodes left in the tree:
var binaryTreeNode = function (value) {
return {
value : value,
left : null,
right : null
};
};
var binarySearchTree = function () {
var tree = Object.create( binarySearchTreeMethods );
tree.root = null;
return tree;
};
var binarySearchTreeMethods = {
insert: function (value, node) {
var newNode = binaryTreeNode( value );
// check if tree is empty
if ( this.isEmpty() ) {
this.root = newNode;
return;
}
// initialize node
if ( node === void 0 ) node = this.root;
// pare value with node.value
if ( value <= node.value ) {
// check if left exists
if ( node.left ) {
this.insert( value, node.left );
} else {
node.left = newNode;
}
} else {
if ( node.right ) {
this.insert( value, node.right );
} else {
node.right = newNode;
}
}
},
remove: function (value, node) {
var nextRightValue, nextLeftValue, minRight;
if ( !this.isEmpty() ) {
// initialize node
if ( node === void 0 ) node = this.root;
// pare the node's value with the value
if ( value < node.value ) {
// check if there is a left node
if ( node.left ) {
node.left = this.remove( value, node.left );
}
} else if ( value > node.value ) {
// check if there is a right node
if ( node.right ) {
node.right = this.remove( value, node.right );
}
} else {
// at this point, value === node.value
// check if node is a leaf node
if ( node.left === null && node.right === null ) {
// edge case of single node in tree (i.e. root node)
if ( this.getHeight() === 0 ) {
this.root = null;
return this.root;
} else {
node = null;
}
} else if ( node.left === null ) {
node = node.right;
} else if ( node.right === null ) {
node = node.left;
} else {
// node has both left and right
minRight = this.findMinValue( node.right );
node.value = minRight;
node.right = this.remove( minRight, node.right );
}
}
return node;
}
},
contains: function (value, node) {
if ( this.isEmpty() ) return false;
// tree is not empty - initialize node
if ( node === void 0 ) node = this.root;
// check if node's value is the value
if ( value === node.value ) return true;
if ( value < node.value ) {
// check if left node exists
return node.left ? this.contains( value, node.left ) : false;
} else {
// check if right node exists
return node.right ? this.contains( value, node.right ) : false;
}
},
findMaxValue: function (node) {
if ( !this.isEmpty() ) {
if ( node === void 0 ) node = this.root;
while ( node.right ) {
node = node.right;
}
return node.value;
}
},
findMinValue: function (node) {
if ( !this.isEmpty() ) {
if ( node === void 0 ) node = this.root;
while ( node.left ) {
node = node.left;
}
return node.value;
}
},
getHeight: function (node) {
if ( !this.isEmpty() ) {
// initialize node
if ( node === void 0 ) node = this.root;
// base case
if ( node.left === null && node.right === null ) return 0;
if ( node.left === null ) return 1 + this.getHeight( node.right );
if ( node.right === null ) return 1 + this.getHeight( node.left );
return 1 + Math.max( this.getHeight( node.left ), this.getHeight( node.right ) );
}
},
isEmpty: function () {
return this.root === null;
}
};
Inserting values into the binary search tree works fine:
var bst = binarySearchTree();
bst.insert(10);
bst.insert(5);
bst.insert(20);
bst.insert(30);
bst.insert(22);
bst.insert(18);
I e across an issue when I start removing the root
value each time:
bst.remove(10); // this works fine and the resulting bst tree is structurally correct
bst.remove(18); // this works fine and the resulting bst tree is structurally correct
bst.remove(20); // this works fine and the resulting bst tree is structurally correct
bst.remove(22); // this works fine and the resulting bst tree is structurally correct
bst.remove(30); // THIS IS WHERE THE ISSUE OCCURS
Prior to removing 30, the tree only has two values: 30 as the root value and 5 as the root.left value. I would expect that removing 30 would give me a tree which has 5 as the root. However, removing 30 doesn't do anything to the tree; it remains the same.
Further testing shows that if I had removed 5 first and then 30, then everything works fine as well:
bst.remove(10); // this works fine and the resulting bst tree is structurally correct
bst.remove(18); // this works fine and the resulting bst tree is structurally correct
bst.remove(20); // this works fine and the resulting bst tree is structurally correct
bst.remove(22); // this works fine and the resulting bst tree is structurally correct
bst.remove(5); // Results in a tree with 30 as the root value
bst.remove(30); // Results in the empty tree where root === null
Can anyone help me understand why removing 30 initially didn't work?
Here's my implementation for a Binary Search Tree in JavaScript. All functions appear to be working properly except for the remove
function. Specifically, it seems to be removing nodes properly until there's 2 nodes left in the tree:
var binaryTreeNode = function (value) {
return {
value : value,
left : null,
right : null
};
};
var binarySearchTree = function () {
var tree = Object.create( binarySearchTreeMethods );
tree.root = null;
return tree;
};
var binarySearchTreeMethods = {
insert: function (value, node) {
var newNode = binaryTreeNode( value );
// check if tree is empty
if ( this.isEmpty() ) {
this.root = newNode;
return;
}
// initialize node
if ( node === void 0 ) node = this.root;
// pare value with node.value
if ( value <= node.value ) {
// check if left exists
if ( node.left ) {
this.insert( value, node.left );
} else {
node.left = newNode;
}
} else {
if ( node.right ) {
this.insert( value, node.right );
} else {
node.right = newNode;
}
}
},
remove: function (value, node) {
var nextRightValue, nextLeftValue, minRight;
if ( !this.isEmpty() ) {
// initialize node
if ( node === void 0 ) node = this.root;
// pare the node's value with the value
if ( value < node.value ) {
// check if there is a left node
if ( node.left ) {
node.left = this.remove( value, node.left );
}
} else if ( value > node.value ) {
// check if there is a right node
if ( node.right ) {
node.right = this.remove( value, node.right );
}
} else {
// at this point, value === node.value
// check if node is a leaf node
if ( node.left === null && node.right === null ) {
// edge case of single node in tree (i.e. root node)
if ( this.getHeight() === 0 ) {
this.root = null;
return this.root;
} else {
node = null;
}
} else if ( node.left === null ) {
node = node.right;
} else if ( node.right === null ) {
node = node.left;
} else {
// node has both left and right
minRight = this.findMinValue( node.right );
node.value = minRight;
node.right = this.remove( minRight, node.right );
}
}
return node;
}
},
contains: function (value, node) {
if ( this.isEmpty() ) return false;
// tree is not empty - initialize node
if ( node === void 0 ) node = this.root;
// check if node's value is the value
if ( value === node.value ) return true;
if ( value < node.value ) {
// check if left node exists
return node.left ? this.contains( value, node.left ) : false;
} else {
// check if right node exists
return node.right ? this.contains( value, node.right ) : false;
}
},
findMaxValue: function (node) {
if ( !this.isEmpty() ) {
if ( node === void 0 ) node = this.root;
while ( node.right ) {
node = node.right;
}
return node.value;
}
},
findMinValue: function (node) {
if ( !this.isEmpty() ) {
if ( node === void 0 ) node = this.root;
while ( node.left ) {
node = node.left;
}
return node.value;
}
},
getHeight: function (node) {
if ( !this.isEmpty() ) {
// initialize node
if ( node === void 0 ) node = this.root;
// base case
if ( node.left === null && node.right === null ) return 0;
if ( node.left === null ) return 1 + this.getHeight( node.right );
if ( node.right === null ) return 1 + this.getHeight( node.left );
return 1 + Math.max( this.getHeight( node.left ), this.getHeight( node.right ) );
}
},
isEmpty: function () {
return this.root === null;
}
};
Inserting values into the binary search tree works fine:
var bst = binarySearchTree();
bst.insert(10);
bst.insert(5);
bst.insert(20);
bst.insert(30);
bst.insert(22);
bst.insert(18);
I e across an issue when I start removing the root
value each time:
bst.remove(10); // this works fine and the resulting bst tree is structurally correct
bst.remove(18); // this works fine and the resulting bst tree is structurally correct
bst.remove(20); // this works fine and the resulting bst tree is structurally correct
bst.remove(22); // this works fine and the resulting bst tree is structurally correct
bst.remove(30); // THIS IS WHERE THE ISSUE OCCURS
Prior to removing 30, the tree only has two values: 30 as the root value and 5 as the root.left value. I would expect that removing 30 would give me a tree which has 5 as the root. However, removing 30 doesn't do anything to the tree; it remains the same.
Further testing shows that if I had removed 5 first and then 30, then everything works fine as well:
bst.remove(10); // this works fine and the resulting bst tree is structurally correct
bst.remove(18); // this works fine and the resulting bst tree is structurally correct
bst.remove(20); // this works fine and the resulting bst tree is structurally correct
bst.remove(22); // this works fine and the resulting bst tree is structurally correct
bst.remove(5); // Results in a tree with 30 as the root value
bst.remove(30); // Results in the empty tree where root === null
Can anyone help me understand why removing 30 initially didn't work?
Share Improve this question edited Jan 7, 2015 at 4:38 RobG 147k32 gold badges179 silver badges214 bronze badges asked Jan 7, 2015 at 4:23 wmockwmock 5,4924 gold badges43 silver badges62 bronze badges 6-
Consider not initialising left and right as in
left : null
, so when inserting, instead ofif ( node.left )
you'd haveif ('left' in node)
, otherwise if the value is 0 you might get strange results. Same when removing: if there is no left, remove the property. Not to sure aboutnode === void 0
, why notnode === undefined
? Ok, it's 3 more characters to type but more expressive. – RobG Commented Jan 7, 2015 at 5:01 -
1
@RobG Even if
node.left.value
is 0,node.left
will always be truthy ifnode.left
is present. – JLRishe Commented Jan 7, 2015 at 5:17 -
I would also say that having the having a BST that deletes its nodes'
.left
and.right
properties in order to clear them would be pretty weird in terms of OOP design. Most of the world's BSTs don't do that (most languages don't even allow you to do that). – JLRishe Commented Jan 7, 2015 at 6:21 -
@JLRishe—then why does it return false if
node.left
is null? Or undefined? Or false? See ECMA-262 §12.5 step 2. Javascript isn't "most languages". Why have a left property if there is no left node? Makes sense to me. Using null to indicate no left node means that the tree can't store a value of null. I don't see where the OP has limited the tree to certain values. – RobG Commented Jan 7, 2015 at 8:27 -
@RobG I don't understand your first question or why you referred me to the JS spec for
if
statement. Why does what return false? You seem to be unfamiliar with the typical design of binary trees (and misunderstanding OP's implementation). The tree can store a value of null, or any other value. The tree consists of a hierarchy ofbinaryTreeNode
s, and each of those nodes can contain any value the user desires (although I'm not sure how much sense it makes to store a null value in a binary search tree). – JLRishe Commented Jan 7, 2015 at 8:52
3 Answers
Reset to default 10Your code has a provision for the case when the found node is the root and it is the only node in the tree, and if a node has both a left and a right child, you overwrite its value. But when the node to remove is the root and it has only one child, there is nothing in your code that overwrites this.root
, and you don't overwrite the root's value, so it is not removed and the tree remains unmodified.
You can fix this by changing this:
if ( node === void 0 ) node = this.root;
// pare the node's value with the value
if ( value < node.value ) {
to this:
if ( node === void 0 ) {
this.root = this.remove(value, this.root);
// pare the node's value with the value
} else if ( value < node.value ) {
Once that is fixed, you can simplify your logic a bit:
remove: function (value, node) {
if (!this.isEmpty()) {
// initialize node
if (!node) {
this.root = this.remove(value, this.root);
} else if (value < node.value && node.left) {
node.left = this.remove(value, node.left);
} else if (value > node.value && node.right) {
node.right = this.remove(value, node.right);
} else if (value === node.value) {
// check if node is a leaf node
if (node.left && node.right) {
// node has two children. change its value to the min
// right value and remove the min right node
node.value = this.findMinValue(node.right);
node.right = this.remove(node.value, node.right);
} else {
// replace the node with whichever child it has
node = node.left || node.right;
}
}
return node;
}
},
and then you can simplify it further by separating it into two methods:
remove: function (value) {
this.root = this._removeInner(value, this.root);
},
_removeInner: function (value, node) {
if (node) {
if (value < node.value) {
node.left = this._removeInner(value, node.left);
} else if (value > node.value) {
node.right = this._removeInner(value, node.right);
} else if (node.left && node.right) {
node.value = this.findMinValue(node.right);
node.right = this._removeInner(node.value, node.right);
} else {
node = node.left || node.right;
}
}
return node;
},
Demo
@wmock has asked how I went about solving this problem so I'll elaborate on that a bit.
The first thing I did was walk the code in the debugger, focusing on the bst.remove(30)
part. I noticed that 30 was the root at that point and that it remained there after remove()
was done. This led me to noticing that the code never modifies the root in that particular case.
I then looked at how the return value of this.remove()
was being assigned to node.left
and node.right
, and with some recollection of BST algorithms, thought it would make sense to emulate that for the root as well. And that was indeed the answer.
There were a few things that motivated splitting the method into two methods:
- I noticed that the method had a fair amount of special-case functionality that was only relevant for the initial call to
bst.remove()
- Checking
this.isEmpty()
- Using
this.root
for the value ofnode
ifnode
was null - Resetting
this.root
to null under certain cases when the tree height is 0
- Checking
It seemed sloppy to be doing all of that in every pass through remove()
- I also repeatedly found myself wanting to use
if (!node)
to check whether I had reached the edge of the tree, but I couldn't because there was that special case logic to usethis.root
whennode
was null.
Splitting the method into two parts resolved all of the above issues.
Note that in a lot of BST implementations, the functionality in _removeInner()
would be a method on the binaryTreeNode
type, and the tree would just interact with the root node. That eliminates the need to pass a node from one method call to the next:
In binarySearchTree
:
remove: function (value) {
this.root && this.root.remove(value);
},
In binaryTreeNode
:
remove: function (value) {
if (value < this.value) {
this.left = this.left && this.left.remove(value);
} else if (value > this.value) {
this.right = this.right && this.right.remove(value);
} else if (this.left && this.right) {
this.value = this.right.findMinValue();
this.right = this.right.remove(this.value);
} else {
return this.left || this.right;
}
return this;
},
findMinValue: function () {
return this.left ? this.left.findMinValue() : this.value;
}
Demo
Here is the full example of a Binary tree with insert and remove functionalities
function Node(val) {
this.data = val;
this.right = null;
this.left = null;
}
function BST() {
this.root = null;
this.insert = insert;
this.inOrder = inOrder;
this.remove = remove;
this.removeNode = removeNode;
this.kthSmallestNode = kthSmallestNode;
}
function insert(val) {
if (val == null || val == undefined)
return;
if (this.root == null) {
this.root = new Node(val);
return;
}
var current = this.root
var newNode = new Node(val);
while (true) {
if (val < current.data) {
if (current.left == null) {
current.left = newNode;
return;
}
current = current.left;
} else {
if (current.right == null) {
current.right = newNode;
return;
}
current = current.right;
}
}
}
function remove(val) {
this.root = removeNode(this.root, val);
}
function removeNode(current, value) {
if (value == null || value == undefined)
return;
if (value == current.data) {
if (current.left == null && current.right == null) {
return null;
} else if (current.left == null)
return current.right;
else if (current.right == null)
return current.left;
else {
var tempNode = kthSmallestNode(current.right);
current.data = tempNode.data;
current.right = removeNode(current.right, tempNode.data);
return current;
}
} else if (value < current.data) {
current.left = removeNode(current.left, value);
return current;
} else {
current.right = removeNode(current.right, value);
return current;
}
}
function kthSmallestNode(node) {
while (!(node.left == null))
node = node.left;
return node;
}
function inOrder(node) {
if (!(node == null)) {
inOrder(node.left);
console.log(node.data + " ");
inOrder(node.right);
}
}
var tree = new BST();
tree.insert(25);
tree.insert(20);
tree.insert(30);
tree.insert(27);
tree.insert(21);
tree.insert(16);
tree.insert(26);
tree.insert(35);
tree.remove(30)
console.log("Inorder : ")
console.log(tree.inOrder(tree.root))
Good Luck!!!
I have a pretty simplified answer that I think most people will understand and it takes into account children nodes. The key is that if you are removing a value with a right and left child that you first go left and then all the way right because this assures you that it will have no children and be easier to update.
removeNode(val) {
let currentNode, parentNode, nextBiggestParentNode=null, found=false, base=[this.root];
while(base.length > 0 && !found) {
currentNode = base.pop();
if(currentNode.value === val) {
found=true;
if(!currentNode.left && !currentNode.right) {
parentNode.right === currentNode ? parentNode.right = null : parentNode.left = null;
}
else if(!currentNode.right && currentNode.left) {
parentNode.right === currentNode ? parentNode.right = currentNode.left : parentNode.left = currentNode.left;
}
else if(!currentNode.left && currentNode.right) {
parentNode.right === currentNode ? parentNode.right = currentNode.right : parentNode.left = currentNode.right;
}
else {
let _traverse = node => {
if (node.right) {
nextBiggestParentNode = node;
_traverse(node.right);
}
else {
currentNode.value = node.value;
nextBiggestParentNode ? nextBiggestParentNode.right = null : currentNode.left = null;
}
}
_traverse(currentNode.left);
}
}
else {
parentNode = currentNode;
val > currentNode.value && currentNode.right ? base.unshift(currentNode.right) : base.unshift(currentNode.left);
}
}
return this;
}
that code is part of a class, here is the rest of my constructor code if anybody is interested
let TreeNode = class {
constructor(value, left=null, right=null) {
this.value = value;
this.left = left;
this.right = right;
}
}
let BST = class {
constructor(root=null) {
this.root = root;
}
insert(nodeToInsert) {
if (this.root === null) {
this.root = nodeToInsert;
} else {
this._insert(this.root, nodeToInsert);
}
}
_insert(root, nodeToInsert) {
if (nodeToInsert.value < root.value) {
if (!root.left) {
root.left = nodeToInsert;
} else {
this._insert(root.left, nodeToInsert);
}
} else {
if (!root.right) {
root.right = nodeToInsert;
} else {
this._insert(root.right, nodeToInsert);
}
}
}
here is some demo code to create a bst and remove a value
let bst = new BST();
const nums = [20,10,5,15,3,7,13,17,30,35,25,23,27,37,36,38];
function createBst() {
for (let i of nums) {
bst.insert(new TreeNode(i));
}
console.log(JSON.stringify(bst, null, 2));
bst.removeNode(35);
}
createBst();
console.log(JSON.stringify(bst, null, 2));