views:

76

answers:

2

Hi,

I'm trying to extract the minimum from a binary heap but it's not working. Here's my BubbleDown code:

void heapBubbleDown(Heap * const heap, int idx) {
    int min;

    while(RIGHT(idx) < heap->count) {
        min = LEFT(idx);

        if(RIGHT(idx) < heap->count) {
            if(heap->items[LEFT(idx)] > heap->items[RIGHT(idx)]) {
                min = RIGHT(idx);
            }
        }

        heapSwapValue(&(heap->items[idx]), &(heap->items[min]));

        idx = min;
    }
}

It looks like it only swaps a few numbers but not all of them, I can't understand why. I tried to recode it differently and many times already...

What am I doing wrong?

+2  A: 

I don't think the problem is that it swaps to few elements. You have to stop when the smallest child >= current item.

I would rewrite the last two lines to:

    if (heap->items[idx] > heap->items[min]) {
       heapSwapValue(&(heap->items[idx]), &(heap->items[min]));
       idx = min;
    } 
    else
       break;
}
Henk Holterman
Thanks... I don't like to use breaks though, I'm thinking of adding a variable to the while condition, but is there any other way? I can't think of any though...
Nazgulled
@Nazgul, not easy. You have to do all of the left/right stuff before you can know. You can keep a boolean flag 'noptInOrderYet', but I would use a break.
Henk Holterman
I prefer not to use a break, but thanks for the insight.
Nazgulled
+2  A: 

The condition of the while is not sufficient. It may be the case that there is no right child but you need to do the swap with the left child. Moreover as Henk suggests, you need to check if the calculated minimum is in fact smaller than your current value.

tafa
tafa, yes, I missed that. It should be `while (LEFT(idx) < heap->count)`
Henk Holterman