patternjavaMinor
Binary Search Tree insert method (map interface)
Viewed 0 times
mapinsertsearchmethodinterfacebinarytree
Problem
This is my implementation based on
What do you think? There's something to improve?
Map interface. The BST is a linked binary tree with root reference, both internal and external nodes (MyBSNode) contains (key, value) entriesWhat do you think? There's something to improve?
public BSPosition insert(int k, E e) {
if(isEmpty()){
/* Sets root */
BSPosition r = new MyBSNode(null);
r.setElement(k, e);
setRoot(r);
size++;
return r;
}
BSPosition n = treeInsert(k, root); //k-key node or last node visited
if(k == n.getKey()){ //tree has already a k-key node
n.setElement(k, e); //only modifies e
return n;
}
/* n is a external (last visited) node with different key */
size++;
if(k treeInsert(int k, BSPosition v){
if(isLeaf(v)) return v;
if(k < v.getKey()){
if(v.getLeftChild() == null) return v;
return treeInsert(k, v.getLeftChild());
}
else if(k == v.getKey()) return v;
else{
if(v.getRightChild() == null) return v;
return treeInsert(k, v.getRightChild());
}
}Solution
The algorithm looks fine. Some other notes:
-
I would use longer variable names than
-
Consider creating a default contructor, a named factory method or an explanatory local variable for
-
-
Creating a
-
I'd consider moving
-
-
I would use longer variable names than
r, k, e etc. Longer names would make the code more readable since readers/maintainers don't have to decode or memorize the abbreviations.-
new MyBSNode(null)Consider creating a default contructor, a named factory method or an explanatory local variable for
null. Currently readers have to check the MyBSNode constructor to figure out what's that null supposed to mean.-
MyBSNode does not seem to have a good name. Try to find something more descriptive.-
n.setElement(k, e); // only modifies eCreating a
setValue(E value) method would make the comment unnecessary and would be cleaner.-
I'd consider moving
isLeaf to BSPosition since it seems data envy. I guess its only task is to check that both left and right children are nulls.-
treeInsert does not do any insertion, its name is a little bit misleading, it just searches the parent of the new node or a node with the same key. I'd call it according to that. (searchInsertionPoint for example.)Code Snippets
new MyBSNode<E>(null)n.setElement(k, e); // only modifies eContext
StackExchange Code Review Q#30519, answer score: 2
Revisions (0)
No revisions yet.