Adding or removing elements from a min heap

Question

I am trying to create a min heap but I am running into the issue where the numbers that are being displayed in my min heap are all in random order and there are extra 0's where there should be different values. This is the code for my class that does most of the work:

public class Heap211 {
static Random rand = new Random();
static public int[] Heap;
static public int size;

Heap211(){
    Heap = new int[30];
    size = 0;
}
static public int parent(int index){//location of parent
    return index / 2;//array[k / 2]
}
static public int leftChild(int index){//location of left child
    return index * 2;//array[k * 2]
}
static public int rightChild(int index){//location of right child
    return index * 2 + 1;//array[k * 2 + 1]
}
static public boolean hasParent(int index){
    return index > 1;
}
static public boolean hasLeftChild(int index){
    return leftChild(index) * 2 <= size;
}
static public boolean hasRightChild(int index){
    return rightChild(index * 2) + 1 <= size;
}
static public void swap(int[] a, int index1, int index2){//swaps nodes
    int temp = a[index1];
    a[index1] = a[index2];
    a[index2] = temp;
}
static public int peek(){//peeks at the top of the stack (min value)
    return Heap[1];
}
public static boolean isEmpty(){
   return size == 0;
}
static int randInt(int min, int max){//generates random int between two numbers 
    return ((int) (Math.random()*(max - min))) + min; 
}
public String toString(){
    String result = "[";
    if(!isEmpty()){
        result += Heap[1];
        for(int i = 2; i <= size; i++){
            result += ", " + Heap[i];
        }
    }
    return result + "]";
}
public void add(int value){//adds the give value to this priority queue in order
    if(size + 1 >= Heap.length){
        Heap = Arrays.copyOf(Heap, Heap.length * 2);
    }
    size++;
    Heap[size + 1] = value;//add as rightmost leaf

    //"bubble up" as necessary to fix ordering
    int index = size + 1;
    boolean found = false;
    while(!found && hasParent(index) && hasLeftChild(index)){
        int parent = parent(index);
        if(Heap[index] < Heap[parent]){
            swap(Heap, index, parent(index));
            index = parent(index);
        }else{//after done bubbling up
            found = true;
        }
    }
}
public int remove(){
    //move rightmost leaf to become new root
    int result = peek();//last leaf -> root
    Heap[1] = Heap[size];
    size--;

    //"bubble down" as necessary to fix ordering
    int index = 1;
    boolean found = false;
    while(!found && hasLeftChild(index)){
        int left = leftChild(index);
        int right = rightChild(index);  
        int child = left;
        if(hasRightChild(index) && Heap[right] < Heap[left]){
            child = right;
        }
        if(Heap[index] > Heap[child]){
            swap(Heap, index, child);
            index = child;
        }else{
            found = true;//found proper location, stop the loop
        }
    }
    return result;
}

This is the code for my main class:

    public static void main(String[] args){
    Heap211 pq = new Heap211();
    for(int node = 1;node <= 30; node++){//loop runs 30 times for 30 nodes
        int smValue = randInt(0,2);//generates random number between 1 and 0
        if(smValue == 0){//if random number is 0 then it will add random number to heap
            int value = randInt(0,100);//generates random number between 0 and 100
            System.out.println(node + " Add " + value + ": ");
            pq.add(value);//adds random number
            System.out.println(pq);//print heap

        }else if(smValue == 1 && pq.isEmpty()){
            int value = pq.remove();
            System.out.println(node + " Remove " + value + ": ");
            System.out.println(pq);
        }
    }

I have a GUI that displays all the numbers but I am getting the wrong output. Any helpful pointers would be greatly appreciated! Thanks.

Answer 1

I found a few problems in your code.

Your hasLeftChild function is wrong. You have return leftChild(index*2) <= size;. But you really should be checking for leftChild(index) <= size. You have a similar error in your hasRightChild function.

Not sure why you pass an array parameter to swap. The only array in which you swap stuff is the Heap array, which is a member of the class.

You have an error in your add method. You increment the size, and then add an item. That is:

size++;
Heap[size + 1] = value;

So imagine what happens when you add the first item. size is equal to 0, and you increment it to 1. Then you add the value at index size+1. So your array contains [0, 0, value]. That's probably the source of your extra 0's. I think what you want is:

Heap[size] = value;
size++;

You'll have to modify the rest of your code to take that into account.

Your "bubble up" loop is kind of wonky. You have:

while (!found && hasParent(index) && hasLeftChild(index))

That's never going to bubble anything up, because when you add something to the last element of the heap, that node doesn't have a left child. You also don't need the found flag. You can write:

while (hasParent(index) && Heap[index] < Heap[parent]]) {
    swap(Heap, index, parent(index));
    index = parent(index);
}

I can't guarantee that those are the only errors in your code, but they're the ones I found in a quick review of your code.

On a general note, why in the world are you creating a 1-based binary heap in a language that has 0-based arrays? There's no need to do that, and it's confusing as heck. For why I think it's a bad idea, see https://stackoverflow.com/a/49806133/56778 and http://blog.mischel.com/2016/09/19/but-thats-the-way-weve-always-done-it/.

Finally, you should learn to use your debugger, as suggested in comments. Take the time to do it now. It will save you hours of frustration.