Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add solution for BinarySearchTree #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions Session08/binarytree/solutions/BinarySearchTree.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// This class fails to compile, can you figure out why, and fix it?
// This is beacuse BinaryTree has a constructor which is not default. Hence, when we extend this class and try to call a no-argument constructor, it'll give a compile-time error. so, we call it by passing the parameters (to instantiate the root node) in super() called inside the constructor of BinarySearchTree
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use multiline comments for long comments.

/* line1
 * line2
 */


class BinarySearchTree extends BinaryTree {

private int flag = 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this is not a boolean? Also, why is it a member variable? What does "BinarySearchTree.flag" mean for the tree as a whole?


public BinarySearchTree()
{
super(0);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the root randomly 0? It will be extremely unexpected if a user constructs an "empty" binary search tree, but minValue or maxValue will return 0.

}

// insert node
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment really necessary? The function name is insert. Self-documenting code.

@Override
public boolean insert(int value) {
super.root = insert(super.root, value);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the node already exists? This function always returns true regardless of whether the insert "fails" or succeeds.

return true;
}

private TreeNode insert(TreeNode root, int data)
{
// if current node is null, insert node with data
if (root == null) root = new TreeNode(data);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both your if and else blocks have a single line, this formatting is fine, like the way you have used below.

However, in this case, please adhere to the repo's coding style of

if (<condition>) {
}
else {
}

Curly braces are good. Always use them, even when they are not needed.

else
{
// traverse to left sub-tree if value to be inserted is lesser than value in current node
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment says "lesser than", but your code says "lesser than or equal to". Decide which one is incorrect.

if (data <= node.getData()) root.left = insert(root.left, data);
// traverse to right sub-tree if value to be inserted is greater than value in current node
else root.right = insert(root.right, data);
}
return root;
}

// delete node
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment really necessary? The function name is delete. Self-documenting code.

@Override
public boolean delete(int value) {
super.root = deleteRec(super.root, value);
if(flag==1) {flag=0;return false;}
Copy link
Owner

@havanagrawal havanagrawal Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please format as

if (flag == 1) {
    flag = 0;
    return false;
}

Note the spaces

else return true;
}

TreeNode deleteRec(TreeNode root, int key)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow code style of opening the curly brace on the same line.

{
// if tree is empty, set flag to 1 and return root
if (root == null) {flag=1;return root;}

// traverse to left sub-tree if value to be inserted is lesser than value in current node
if (key < root.data) root.left = deleteRec(root.left, key);
// traverse to right sub-tree if value to be inserted is greater than value in current node
else if (key > root.data) root.right = deleteRec(root.right, key);

// if value is same as current value, delte it
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in comment, should be "delete"

else
{
// node with only one child or no child
if (root.left == null) return root.right;
else if (root.right == null) return root.left;

// if node has two children, get the lowest inorder successor from the right subtree
root.data = minValue(root.right);
// Delete the inorder successor
root.right = deleteRec(root.right, root.data);
}
return root;
}

// find the lowest value in the tree by reaching the leftmost leaf node
int minValue(TreeNode root)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the tree is empty?

{
int minv = root.data;
while (root.left != null)
{
minv = root.left.data;
root = root.left;
}
return minv;
}

// check if node exists
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment really necessary? The function name is exists. Self-documenting code.

@Override
public boolean exists(int find) {
TreeNode tn = search(super.root,find);
if(flag==1) {flag=0;return false;}
else return true;
}

public TreeNode search(TreeNode root, int key)
{
// not found
if(root==null)
{
flag=1;
return root;
}
// if found, return root
if (root.data==key) return root;

// traverse to left sub-tree if value to be inserted is lesser than value in current node
if (root.data > key) return search(root.left, key);
// traverse to right sub-tree if value to be inserted is greater than value in current node
return search(root.right, key);
}

// lowest common ancestor
@Override
public TreeNode lowestCommonAncestor(int value1, int value2) {

TreeNode tn = lca(super.root, value1,value2);
if(flag==1) return null;
else return tn;
}

// the node value present in between a and b is the lca. Assuming, a<b
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why assume that a < b? Why not ensure it?

public TreeNode lca(TreeNode root, int a, int b)
{
if(root == null) {flag=1;return null;}

if(root.data>b && root.data>a) lca(root.left,a,b);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have assumed that a < b, is it really necessary to check if root.data is greater than both of them?

else if(root.data<b && root.data<a) lca(root.right,a,b);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have assumed that a < b, is it really necessary to check if root.data is lesser than both of them?


return root;
}
}
79 changes: 79 additions & 0 deletions Session08/binarytree/solutions/BinaryTree.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
abstract class BinaryTree {
protected TreeNode root;

/* While this constructor cannot be called directly,
it is useful because the child classes can use it,
by using super(), to instantiate the root node
*/
public BinaryTree(int rootValue) {
this.root = new TreeNode(rootValue);
}

public void inOrderTraversal() {
//TODO
inorderRec(root);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming that inOrderRec() stands for inOrderRecursive. However, there is no way for me to know this, since it's not a convention. Please name it inOrderRecursive

}

public void preOrderTraversal() {
//TODO
preorderRec(root);
}

public void postOrderTraversal() {
//TODO
postorderRec(root);
}

// inorder traversal - left,visit,right
void inorderRec(TreeNode root) {
if (root != null) {
inorderRec(root.left);
System.out.println(root.data);
inorderRec(root.right);
}
}

// preorder traversal - visit,left,right
void preorderRec(TreeNode root) {
if (root != null) {
System.out.println(root.data);
inorderRec(root.left);
inorderRec(root.right);
}
}

// postorder traversal - left,right,visit
void postorderRec(TreeNode root) {
if (root != null) {
inorderRec(root.left);
inorderRec(root.right);
System.out.println(root.data);
}
}

/*NOTE: Think about why these methods are abstract.
Why can't we just implement them like we did for the traversals?
*/

/* Insert a node with the given value into the tree
Return true if the insert succeeds, else return false.

This is a signature similar to the one we saw in the CFW.
*/
public abstract boolean insert(int value);

/* Delete the node with the given value from this tree, and
return the deleted node. Return null if the value doesn't exist.
*/
public abstract boolean delete(int value);

/* Return true if the given value exists in the tree
*/
public abstract boolean exists(int find);

/* Return the node that is the LCA of the two given values.

Return null if either or both of the values doesn't exist in the tree.
*/
public abstract TreeNode lowestCommonAncestor(int value1, int value2);
}
43 changes: 43 additions & 0 deletions Session08/binarytree/solutions/TreeNode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
class TreeNode {
TreeNode left;
TreeNode right;

int data;

public TreeNode(int data) {
this.data = data;
this.left = null;
this.right = null;
}

// set left node
public void setLeft(TreeNode n)
Copy link
Owner

@havanagrawal havanagrawal Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really bad! We do not want setters on a TreeNode. Imagine what happens if I insert a TreeNode into a BinarySearchTree, and then set its value to be something else. Now you've violated the BinarySearchTree invariant.

Additionally, member variables without an explicit access modifier are default, and have package level access. Thus we don't really need getters either.

(In the ideal case, the TreeNode class would be immutable.)

{
this.left = n;
}
// set right node
public void setRight(TreeNode n)
{
this.right = n;
}
// get left node
public TreeNode getLeft()
{
return this.left;
}
// get right node
public TreeNode getRight()
{
return this.right;
}
// set data to node
public void setData(int d)
{
this.data = d;
}
// get data from node
public int getData()
{
return data;
}
}