Program throwing an exception after I check for the exception

Question

I must be missing something very simple because this is blowing my mind!

I am trying to implement a Heap using a CompleteBinaryTree (which is implemented using an array). This CompleteBinaryTree is an array of Position<T> where each Position holds an element. I am writing the add(T t) method for the Heap, where t is inserted into the next free position of the CompleteBinaryTree, and then an upheap process is performed until the CompleteBinaryTree is ordered. Here is the method:

    private CompleteBinaryTree<T> tree = new CompleteBinaryTree<T>();
    private Position<T> entry = null;

    public void add(T t) {
        entry = tree.add(t);

        if (entry.equals(tree.root())) {
            return;
        }

        //continue to swap inserted element with its parent   
        //while it is smaller than its parent
        while (entry.element().compareTo(tree.parent(entry).element()) < 0) {
            Position<T> parent = tree.parent(entry);
            Position<T> temp = entry;
            entry = parent;
            parent = temp;
        }
    }

The first element is added fine into the Heap, but when I try to add the second element, an InvalidPositionException is thrown at the while() line. This is where the exeption is being thrown from inside the CompleteBinaryTree class:

    public Position<T> parent(Position<T> p) {
        if (p == root()) throw new InvalidPositionException();

        return array[((ArrayPosition) p).index/2];
    }

And here are the two other methods used from CompleteBinaryTree:

    public Position<T> root() {
        if (isEmpty()) throw new InvalidPositionException();
        return array[1];
    }

    public Position<T> add(T t) {
        if (last == array.length) {
            // extend array
            ArrayPosition[] temp = (ArrayPosition[]) new Object[array.length*2];
            for (int i=1; i < array.length; i++) {
                temp[i] = array[i];
            }
            array = temp;
        }

        array[last] = new ArrayPosition(last, t);
        return array[last++];
    }

How am I getting an exception thrown because p == root(), when I first check if p is the root?

EDIT

Here is the CompleteBinaryTree toString(), which is returned by the Heap toString():

public String toString() {
    StringBuffer buf = new StringBuffer();

    for (int i = 1; i < last; i++) {
        buf.append(" ").append(array[i]);
    }

    return buf.toString();      
}

but before the exception is thrown, the method returns if `entry` is the root, but then the exception is thrown because `entry` is the root? — KOB, Apr 01 '16 at 17:42
Did you debug? Was exception raised on `if (p == root())` or `if (isEmpty())`, better that you move `throw new InvalidPositionException();` to new line — An Do, Apr 01 '16 at 17:43

score 2 · Accepted Answer · answered Apr 01 '16 at 17:52

2

How am I getting an exception thrown because p == root(), when I first check if p is the root?

But you do not check every tree.parent() argument to see whether it is root. You check only the argument passed to the first invocation of that method. Every time the body of the while loop is executed, it sets entry to a new value, and when the loop cycles you pass that new, unchecked, value to tree.parent(). Indeed, each new value of entry is closer to the root than the previous one, because the whole point is to move up the tree from child to parent, i.e. toward the root. It is entirely likely that sometimes this procedure will reach the root, and that's more likely the fewer elements are already in the tree.

One way to solve this would be to move the root-check into the while condition:

while (!entry.equals(tree.root())
        && entry.element().compareTo(tree.parent(entry).element()) < 0) {
    // ...
}

In that case, of course, you do not need the current one-time check that you perform outside the loop.

answered Apr 01 '16 at 17:52

John Bollinger

160,171
8
81
157

Ah yes, that never crossed my mind. That all makes perfect sense, and it appears that everything is working as it should now. Thank you – KOB Apr 01 '16 at 17:59
However, it seems the heap isn't being ordered. When I add elements into the heap and then print the state of the heap, it is ordered in the same fashion in which I inserted the elements into it. Also, no matter what elements I add into the heap - if I print the value of the root after each insertion, it is always the value of the first element I inserted (which makes no sense to me as before it was throwing the exception when the newly inserted element was swapped into the root position). See my original post where I will add the 'toString()' method – KOB Apr 01 '16 at 18:06
@KOB, that additional problem is not the subject of this question, but I nevertheless observe that although your `add()` method traverses the tree from leaf toward root, it does not appear to do anything that reorders any nodes. In particular, code such as `entry = parent` does not do so -- it just changes which node local variable `entry` refers to. It is possible that as you go, you want also to swap the contents of the `Position`s, but that's speculative on my part. – John Bollinger Apr 01 '16 at 18:43
You explained that all very well. I changed it up to swap the values instead and it's all working as it should now. Thank you – KOB Apr 02 '16 at 00:10

Program throwing an exception after I check for the exception

1 Answers1