Maintaining a Min heap on deletion c++

Question

Test case:

8,7,5,2,3,6,9 (NOT min heap) (this is element A* for buildHeap function)

2,3,5,7,8,6,9 (min heap after calling build heap)

3,5,6,7,8,9 (after calling deleteMin) THIS IS INCORRECT

it should be this 3,7,5,9,8,6

I can't seem to find the problem with deleteMin i know my heapify is working but idk maybe im not seeing something.

 Element Heap::deleteMin(Heap& heap){
    Element deleted = heap.H[0];
    heap.H[0] = heap.H[heap.size-1];
    heap.size--;
    cout<<deleted.getKey()<<" has been deleted from heap"<<endl;
    for(int i=heap.capacity/2-1;i>=0;--i)
       heapify(heap,i);
    return deleted;
}

void Heap::heapify(Heap& heap,int index){
    int smallest = 0;
    int left = 2*index;
    int right = 2*index+1;

    if(left < heap.size && heap.H[left].getKey() < heap.H[index].getKey())
        smallest=left;
    else 
       smallest=index;
    if(right < heap.size && heap.H[right].getKey() < heap.H[smallest].getKey())
        smallest=right;
    if(smallest != index){
        int swapKey = heap.H[index].getKey();
        heap.H[index].setKey(heap.H[smallest].getKey());
        heap.H[smallest].setKey(swapKey);
        heapify(heap,smallest);
    }
}


void Heap::buildHeap(Heap& heap, Element* A){
        for(int j=0;j<heap.capacity;++j){
                heap.insert(heap,A[j]);
                for(int i=heap.capacity/2-1;i>=0;--i)
                        heapify(heap,i);
        }
}

We don't know what "heapify" is supposed to do, so we can't can't say anything other than "heapify is doing what it's doing". However, I would note your statement "if(smallest != index)", where you are comparing a value in the array against an index of the array, which certainly doesn't look right - for example, if you added 100 to all the values in the array (so 108,107, 105, etc), then that condition would always return false for an array of 7 elements - it doesn't look right. — racraman, Mar 30 '17 at 09:36
@racraman: `heapify` is a common operation when implementing a heap. It sifts an item down in the heap. It's also commonly called `siftDown`. If you examine `heapify` more closely, you'll see that `smallest` is an index into the array, not a value from the array. — Jim Mischel, Mar 31 '17 at 11:48

score 0 · Answer 1 · answered Mar 30 '17 at 13:31

The first problem is that your calculations for child indexes are wrong. If you're using H[0] as the root of the heap, then

left = (2*index)+1
right = (2*index)+2

The calculations you have assume that the root is at H[1].

The other problem is that you're doing too much work in your deleteMin function:

Element Heap::deleteMin(Heap& heap){
    Element deleted = heap.H[0];
    heap.H[0] = heap.H[heap.size-1];
    heap.size--;
    cout<<deleted.getKey()<<" has been deleted from heap"<<endl;
    for(int i=heap.capacity/2-1;i>=0;--i)
        heapify(heap,i);
    return deleted;
}

After you delete the minimum item and put the last item in the heap at the root, you just need to call heapify(heap, 0); There's no reason to re-build the entire heap in that loop.

So your function becomes:

Element Heap::deleteMin(Heap& heap){
    Element deleted = heap.H[0];
    heap.H[0] = heap.H[heap.size-1];
    heap.size--;
    cout<<deleted.getKey()<<" has been deleted from heap"<<endl;
    heapify(heap, 0);
    return deleted;
}

Your buildHeap method is similarly doing too much work.

You might be interested in a refresher on heaps. My blog article at http://blog.mischel.com/2013/09/29/a-better-way-to-do-it-the-heap/ explains their operation in very simple terms, and my simple heap of integers shows a bare-bones implementation. It's in C# rather than C++, but the code is very similar. You should be able to understand it without trouble.

Maintaining a Min heap on deletion c++

1 Answers1