1

I have a red black tree implementation that I have implemented from pseudo code found in the CLRS and I'm running into a NPE in the insert fixup procedure. Here is my implementation in java. (Ignore the variable names, I know it's not convention)

private RbtNode root;
    private RbtNode nil;
    private static final boolean BLACK = true;
    private static final boolean RED = false;

    public RedBlackTreeImpl() {
        nil = new RbtNode("");
        nil.color = BLACK;
        root = nil;
    }

    @Override
    public void rbInsert(T data) {
        RbtNode z = new RbtNode(data);
        RbtNode y = this.nil;
        RbtNode x = this.root;
        while(x != this.nil){
            y = x;
            if(z.data.toString().compareTo(x.data.toString()) < 0){
                x = x.left;
            } else {
                x = x.right;
            }
        }
        z.parent = y;
        if(y == this.nil) {
            this.root = z;
        } else {
            y.right = z;
        }
        z.left = this.nil;
        z.right = this.nil;
        z.color = RED;
        rbInsertFixup(z);
    }

    @Override
    public void rbInsertFixup(RbtNode z) {
        while(z.parent.color == RED){
            if(z.parent == z.parent.parent.left){
                RbtNode y = z.parent.parent.right;
                if(y.color == RED){
                    z.parent.color = BLACK;
                    y.color = BLACK;
                    z.parent.parent.color = RED;
                    z = z.parent.parent;
                } else if(z == z.parent.right){
                    z = z.parent;
                    leftRotate(z.parent.parent);
                }
                z.parent.color = BLACK;
                z.parent.parent.color = RED;
                rightRotate(z.parent.parent);
            } else {
                RbtNode y = z.parent.parent.left;
                if(y.color == RED){
                    z.parent.color = BLACK;
                    y.color = BLACK;
                    z.parent.parent.color = RED;
                    z = z.parent.parent;
                } else if(z == z.parent.left){
                    z = z.parent;
                    rightRotate(z.parent.parent);
                }
                z.parent.color = BLACK;
                z.parent.parent.color = RED;
                leftRotate(z.parent.parent);
            }
        }
        this.root.color = BLACK;
    }

    @Override
    public void leftRotate(RbtNode x) {
        RbtNode y = x.right;
        x.right = y.left;
        if(y.left != this.nil) {
            y.left.parent = x;
        }
        y.parent = x.parent;
        if(x.parent == this.nil){
            this.root = y;
        } else if(x == x.parent.left){
            x.parent.left = y;
        } else {
            x.parent.right = y;
        }
        y.left = x;
        x.parent = y;
    }

    @Override
    public void rightRotate(RbtNode x) {
        RbtNode y = x.left;
        x.left = y.right;
        if(y.right != this.nil) {
            y.right.parent = x;
        }
        y.parent = x.parent;
        if(x.parent == this.nil){
            this.root = y;
        } else if(x == x.parent.right){
            x.parent.right = y;
        } else {
            x.parent.left = y;
        }
        y.right = x;
        x.parent = y;
    }

Node Class

public class RbtNode<T> {

    T data;
    boolean color;                  //true indicates black; false indicates red
    RbtNode<T> left;
    RbtNode<T> right, parent;

    public RbtNode(T data) {
        if (data == null) {
            throw new IllegalArgumentException("Data cannot be null");
        }
        this.data = data;
    }

    public boolean getColor() {
        return this.color;
    }

    public T getData() {
        return this.data;
    }

    public RbtNode<T> left() {
        return left;
    }

    public RbtNode<T> right() {
        return right;
    }

    public RbtNode<T> parent() {
        return parent;
    }

}

If run a test insert on it like such:

       RedBlackTreeImpl rbt = new RedBlackTreeImpl();
       rbt.rbInsert("a");
       rbt.rbInsert("k");
       rbt.rbInsert("q");
       rbt.rbInsert("c");
       rbt.rbInsert("j");
       rbt.rbInsert("e");

The NPE occurs when attempting to insert "c". Specifically at in rbInsertFixup on the line z.parent.parent.color = RED; immediately before the rightRotate within the first if of the while loop. In that instance z is root, so root's parent is nil, and so it's trying to get nil's parent which is actually null.

I tried making nil's left, right, and parent nodes equal to nil, but that caused an infinite loop. I think this comes down to a lack of concepts issue. So I suppose my question could be broken into an either or question. Is the fact that I'm getting a NPE a sign that I should continue the process, or have I set up nil incorrectly? What is it that I'm missing?

LumberSzquatch
  • 1,054
  • 3
  • 23
  • 46
  • Exception occurs at `z.parent.parent.color = RED;` so obviously `z`, `z.parent`, or `z.parent.parent` is null at that point. – nhouser9 Sep 30 '16 at 00:14
  • How is this an **exact** duplicate? Five minutes was enough to go through my whole post and determine that? – LumberSzquatch Sep 30 '16 at 00:15
  • @nhouser9 Of coarse that is obvious. I got an NPE. I even explained your very comment in my post. That wasn't my question though... – LumberSzquatch Sep 30 '16 at 00:17
  • 1
    @JarrodRoberson This isn't an exact duplicate, he knows what a NPE is and how to debug it. In fact in his code he has shown that he has done the debugging and knows where the null is - he is looking for an explanation on how to correct the code without deviating from the algorithm he found documented. This should not have been closed. – nhouser9 Sep 30 '16 at 00:21
  • @nhouser9 Appreciate your input. Apologies if my comment was a tad bitey. I'm just mildly annoyed with the closing thing. – LumberSzquatch Sep 30 '16 at 00:21
  • I voted to reopen, we'll see what happens. – nhouser9 Sep 30 '16 at 00:22
  • If you had more complete code here - that is, something that one could plop into an IDE and run - and a more exact location of the NPE - then this has a better chance of surviving a reopen. – Makoto Sep 30 '16 at 00:25
  • @Makoto First off, I gave an exact location of the NPE. Not sure how more exact I can get. But I bolded it so it is extra clear what I'm talking about. Secondly I added the node class. That is literally all you need to run this thing, not a whole lot to it. – LumberSzquatch Sep 30 '16 at 00:31
  • When I said "exact location", I was hoping for a snippet of code. However, the code you've provided *does* compile, so I was able to isolate a cause. It's not a vanilla NPE, but it does have some similarities to one. – Makoto Sep 30 '16 at 00:32
  • 2
    Right, so - the question can be answered and isn't the typical NPE. A bit of a concern: things like `parent.parent` is a smell especially if the tree is shallow. The likely error is coming from your not accounting on the fact that the node you want to rotate from the right is only 1 level deep. – Makoto Sep 30 '16 at 00:48
  • Delete the `nil` field and check for `null` instead – Bohemian Sep 30 '16 at 01:00
  • @Bohemian That seemed to complicate things further. If I was doing a normal BST null would be acceptable. But in this case NIL has a color and it should be black. Without that check it causes more abstract NPEs – LumberSzquatch Sep 30 '16 at 01:22
  • @Makoto Are you able to elaborate on what you mean? Shouldn't the color account for that? – LumberSzquatch Sep 30 '16 at 01:23

0 Answers0