Finding Inorder Successor in Binary Search Tree

Question

I'm creating Binary Search Tree class I got all my functions working accept the Successor.

My tree input is 30 10 45 38 20 50 25 33 8 12

When I print Inorder traversal it come up as

8 10 12 20 25 30 33 38 45 50

When I enter value 8 its says its successor is 12. Same for input 10 and 12 itself.

When I enter value 33 it says successor is 50. Which its not.

I can't seem to fix it. Any help of suggestion will be appreciated.

I'm calling it in main like:

BinarySearchTree BST = new BinarySearchTree(array);

BST.getSeccessor(a); // where a is user input to find it successor

Following is my code...

 public TreeNode Successor(TreeNode node) {
        if (node == null) 
         return node; 
        if (node.right != null) {
            return leftMostNode(node.right);
        }
        TreeNode y = node.parent;
        while (null != y && (y.right).equals(node)) {
            node = y;
            y = y.parent;
        }
      return y;
    }

    public static TreeNode leftMostNode(TreeNode node) {
     if (null == node) { return null; }

     while (null != node.left) {
      node = node.left;
     }
     return node;
    }

    /** Search value in Tree need for Successor **/
    public TreeNode Search(int key) {
        TreeNode node = root;                           
        while (node != null) {
            if (key < node.value) {                     
                node = node.left;
            } else if (key  > node.value) {             
                node = node.right;
            } 
            return node;
        }
        return null;
    }

    /** Returns Successor of given value **/
    public int getSuccessor(int val) {
        TreeNode node = Successor(Search(val));
        return node.value; 
    }

Please see solution in java. Feel free to test if it works. – kiruwka Apr 25 '14 at 22:18 — kiruwka, Apr 25 '14 at 22:18

Jonathan E. Landrum · Answer 1 · 2014-04-25T21:40:32.573

3

The inorder successor is going to be

if (node.hasRight())
    node = node.getRight();
else {
    while (node.isRightChild())
        node = node.getParent();
    return node.getParent();
}
while (node.hasLeft())
    node = node.getLeft();
return node;

Might be a good idea to put some null checks in there, too.

edited Apr 25 '14 at 21:40

answered Apr 25 '14 at 21:05

Jonathan E. Landrum

2,748
4
30
46

There is a problem with your `else` case. Consider `node` is the right node. Then by the end of `while` loop you have its parent, and `node=node.getRight` gives you the original node. Instead, you should've looked "higher". – kiruwka Apr 25 '14 at 21:18
Still not fixed, `else` statement is broken. Try to visualize example from my comment. – kiruwka Apr 25 '14 at 21:20
Yep, it's borked for the prefect BST on `{1,2,3,4,5,6,7}`, if you start at `3`, for example. – Niklas B. Apr 25 '14 at 21:21
Now it's borked for the BST on `{1,...,15}` if you start at 7 – Niklas B. Apr 25 '14 at 21:24
@NiklasB. what is `borked` ? You mean broken ? – kiruwka Apr 25 '14 at 21:25
@kiruwka Yes, that's what I mean – Niklas B. Apr 25 '14 at 21:26
@NiklasB. ok. No offense. I think updated solution should work no ? – kiruwka Apr 25 '14 at 21:27
Yep, it's borked. Bigger tree showed it. It needs to be a while loop. – Jonathan E. Landrum Apr 25 '14 at 21:27
@kiruwka Not in the current form, no – Niklas B. Apr 25 '14 at 21:32
OK, I think I may have it now. – Jonathan E. Landrum Apr 25 '14 at 21:42
yep, this is correct. – kiruwka Apr 25 '14 at 21:46
@JonathanLandrum No problem. Sometimes collective thinking is better. – kiruwka Apr 25 '14 at 22:20

Niklas B. · Answer 2 · 2014-04-25T21:39:34.610

3

A possible algorithm would be the following:

If you have a non-empty right subtree, the successor is the smallest value in that subtree
Otherwise, walk up the tree unless you are no longer the right child of your parent or until you reach the root
If you are at the root now, you have no successor. Otherwise, walk up one more step and you are at the successor

Example:

if node.right != null
    return leftmostInSubtree(node.right)
while node != root && node.parent.right == node
    node = node.parent
if node == root
    return null
return node.parent

edited Apr 25 '14 at 21:39

answered Apr 25 '14 at 21:28

Niklas B.

92,950
18
194
224

This is also not correct. last line should be `return node.parent` – kiruwka Apr 25 '14 at 21:38
@kiruwka Good observation :) – Niklas B. Apr 25 '14 at 21:39
ok, finally we have a solution. Last part is to check whether OP keeps track of parent in his implementation : ) – kiruwka Apr 25 '14 at 21:40
@kiruwka OP uses `parent` in his/her own code – Niklas B. Apr 25 '14 at 21:42
1

Ok, cool. +1 one to you both then. – kiruwka Apr 25 '14 at 21:47
I added java code for your solution + fixed some other issues in OP's code. Should work now. – kiruwka Apr 25 '14 at 22:16

score 3 · Accepted Answer · answered Apr 25 '14 at 22:12

I added working solution based on pseudo-code in Niklas answer :

public TreeNode successor(TreeNode node) {
    if (node == null)
        return node;

    if (node.right != null) {
        return leftMostNode(node.right);
    }

    while (null != node.parent /*while we are not root*/ 
            && node.parent.right == node) /* and while we are "right" node */ {

        node = node.parent; // go one level up
    }

    return node.parent;
}

For this to work, you have to fix implementation of some of your methods (provided below) :

/** Search value in Tree need for Successor **/
public TreeNode search(int key) {
    TreeNode node = root;
    while (node != null 
            && key != node.value) {

        if(key < node.value) {
            node = node.left;
        } else if (key > node.value) {
            node = node.right;
        }
    }
    return node;
}

/** Returns Successor of given value **/
public int getSuccessor(int val) {
    TreeNode node; 
    if (null == (node = search(val))
            || (null == (node = successor(node))) {
        // either val is not in BST, or it is the last value-> no successor
        return ERROR_CODE; // -1, for instance;
    }

    return node.value;
}

So I put in exactly what you me and replaced "ERROR_CODE" with -1 because it was showing error. After doing that I input same numbers as mentioned in my original problem and it says successor of 8, 12, 25, 38 is -1? — user3478869, Apr 26 '14 at 01:50
Perhaps you have troubles constructing your tree. I have run this code and everything works fine, you can see it here : https://ideone.com/HpFU1A — kiruwka, Apr 26 '14 at 09:36
Thank you sir! It worked. That's what I was thinking first maybe there is something wrong in my tree but then I though if my In order Traversal come out fine then tree should be fine, but it wasn't. — user3478869, Apr 26 '14 at 15:28

score 0 · Answer 4 · answered Oct 24 '14 at 05:48

I think the above code won't work for the rightmost last node. Here is the improved version:

if(node.right!=null)
{
node=node.right;
while(node.left!=null)
node=node.left;
}`

else if(node.parent==root)
return null;
else if(node==node.parent.left)
node=node.parent;
else
{
while(node.parent!=null)
node=node.parent
}

return node;

Finding Inorder Successor in Binary Search Tree

4 Answers4