C: insert element in empty heap

Question

i'm supposed to write a code, that inserts numbers from stdin into an at first empty max-heap. my code just doesn't get the order of elements right, i found out, that it doesnt even enter the while loop before the third number. Anybody willing to help? Thanks in advance!

int heap_insert(heap* h, int key) {

    if (h->size==MAX_HEAP_SIZE){  
        return(-1);
    }

    h->size=h->size+1;
    int i=h->size-1; 
    h->array[i]=key;                   
    int parent=(i-1)/2;

    while (i>1 && h->array[parent]< key) {
        h->array[i]= h->array[parent];
        i = parent;
        h->array[i]=key;
    }
return(0);
}

Please format/indent your code. There is e.g. a `}` missing at the end. — jofel, Jan 21 '16 at 18:10
I've indented it ... but there's actually more than a missing brace. What does this function return on success? And where? — kdopen, Jan 21 '16 at 18:13
sorry guys, fixed it. it's not returning anything, it's just supposed to fill a heap that has been declared in the main function. — Meowzen, Jan 21 '16 at 18:28
Can we see your heap initializer and fetch functions as well? — Schwern, Jan 21 '16 at 18:29
Commenting on `if (h->size==MAX_HEAP_SIZE)`: in general, bounds checks should be `>=` or `<=`. `if (h->size >= MAX_HEAP_SIZE)` is safer. This protects you if somehow `h->size` jumps the boundary through sloppy coding, memory corruption, or somebody tinkering directly with the heap struct. — Schwern, Jan 21 '16 at 18:37

Schwern · Accepted Answer · 2016-01-21T18:57:31.923

it doesnt even enter the while loop before the third number

That part can be answered. Your loop won't go until i is 2 or greater...

while (i > 1 && h->array[parent]< key) {
       ^^^^^

Here's the code that sets i.

h->size = h->size+1;
int i   = h->size-1;

That code is easier to understand like so:

int i = h->size;
h->size++;

First time through, i will be 0 (assuming h->size is initialized to 0, you didn't show your heap init code). Second time it will be 1. Third time it will be 2 and then finally the loop can run.

I'm guessing you want i >= 1 in the while loop so it will go on the second call.

As for why it's not working, the primary problem is you're forgetting to change parent in the loop.

/* i and parent initialized */
int i=h->size-1;
...
int parent=(i-1)/2;

while (i>1 && h->array[parent]< key) {
    h->array[i]= h->array[parent];

    /* i is changed, but where's parent? */
    i = parent;

    h->array[i]=key;
}

Here's what it should look like. I've changed i, which should only be used in loop indexes, to the more descriptive new.

/* new and parent initialized */
int new = h->size;
...
int parent = (new-1)/2;
while( new != 0 && h->array[parent] < key ) {
    h->array[new] = h->array[parent];
    h->array[parent] = key;

    /* new AND parent changed */
    new = parent;
    parent = (new-1)/2;
}

Here's the complete code, plus I made the heap size dynamic because fixed size structures are a crutch best avoided.

#include <stdio.h>
#include <stdlib.h>

typedef struct {
    int size;
    int max_size;
    int *array;
} heap;

#define INIT_HEAP_SIZE 4

static heap *heap_init() {
    heap *h = calloc(1, sizeof(heap));

    h->max_size = INIT_HEAP_SIZE;
    h->array = calloc(h->max_size, sizeof(int));

    return h;
}

static void heap_destroy(heap *h) {
    free(h->array);
    free(h);
}

static void heap_grow(heap *h) {
    h->max_size *= 2;
    h->array = realloc( h->array, h->max_size * sizeof(int) );
}

static void heap_insert(heap* h, int key) {
    if (h->size >= h->max_size) {
        heap_grow(h);
    }

    int new = h->size;
    h->size++;

    h->array[new] = key;

    int parent = (new-1)/2;
    while( new != 0 && h->array[parent] < key ) {
        h->array[new] = h->array[parent];
        h->array[parent] = key;

        new = parent;
        parent = (new-1)/2;
    }

    return;
}

int main(void) {
    heap *h = heap_init();

    heap_insert(h, 23);
    heap_insert(h, 11);
    heap_insert(h, 42);
    heap_insert(h, 5);
    heap_insert(h, 99);

    for( int i = 0; i < h->size; i++ ) {
        printf("%d: %d\n", i, h->array[i]);
    }

    heap_destroy(h);
}

wow thanks so much! that's way more than i needed haha! but it works now, really appreciate your effort :) — Meowzen, Jan 21 '16 at 19:23

score 0 · Answer 2 · answered Jan 21 '16 at 18:32

It doesn't enter the while loop before the 3rd number because your i is not greater than 1 until the 3rd number is entered. At 1st number i = 0, then 1 then 2.

For the loop, here's my advice on figuring out the problem: Suppose you enter the values 3, 5, 7. As soon as 5 is entered, you need a swap. 5 should become the new root, and 3 should be a child. (So maxheap property is kept) Then, when 7 is entered, another swap is in order. This time with 5. 7 becomes root, 3 and 5 are children. What does this tell you about the indexes? What happens if we insert 10, 16, 1 as well? More swaps? If you answer these properly the while loop should be easy to solve. (Hint: You need to keep swapping by starting from the child, and move to next parent until everything is in order)

C: insert element in empty heap

2 Answers2