0

I was trying to implement Heapsort algorithm in C but I'm not sure I'm done it right. I also have an error "variably modified 'A' at file scope". I was searching similar topic on stackoverflow and I found out that heapSize should be const but then I couldn't use it as a variable in my functions. What should I do?

#include <stdio.h>
#include <math.h>

int heapSize = 100;
int length;
int A[heapSize];

int Heapify(int i) {
  int l = 2*i;
  int r = 2*i + 1;
  int largest;
  int tmp;

  if( l <= heapSize && A[l] > A[i]) largest = l;
  else largest = i;

  if (r <= heapSize && A[r] > A[largest]) largest = r;

  if (largest != i) {
    tmp = A[i];
    A[i] = A[largest];
    A[largest] = tmp;
    }
}

int BuildHeap() {
  heapSize = length;
  int i;

  for (i = floor(length/2); i <= 1; i--) {
    Heapify(i);
    }
}

int main(void) {
  int i,tmp;

  BuildHeap();

  for (i = length; i <= 2; i--) {
    tmp = A[heapSize];
    A[heapSize] = A[1];
    A[1] = A[heapSize];

    heapSize = heapSize - 1;

    Heapify(1);
    }
}

EDIT: I changed the first part of program to:

#include <stdio.h>
#define LENGTH 100


int heapSize = 10;
int A[LENGTH]

and, based on your suggestions, replace the "floor(length/2)" to "LENGTH/2". I solve my problems with compiling but I'm pretty sure this implementation is just bad. Well, anyway thank you very much for your help. I really appreciate it.

5
  • You should make heapSize a const int, and use local variables to do the work. Commented Oct 19, 2015 at 18:31
  • I don't understand. If I'll make heapSize a const, which means I can't change it later, how this algorithm will work? And then why should I use heapsize at all? Wouldn't be better to declare int A[100] for example? Commented Oct 19, 2015 at 18:48
  • It would be better to use a global length and a global pointer, and dynamically allocate whatever space you need. Commented Oct 19, 2015 at 18:50
  • Does length ever change? Also, you don't need to do floor(i/2) like in Javascript when you use integers: integer division truncates, so i / 2 is enough. Commented Oct 19, 2015 at 18:55
  • Thank you, I cut out the floor part. And no, length does not change... and I'm a little confused by it. Well, I guess I'm really bad at this. Commented Oct 19, 2015 at 19:04

1 Answer 1

1

There's lots of issues with your code.

First, you declare a global int length variable, but you never assign it any value. So the length is always ZERO.

In BuildHeap() you do:

    for (i = floor(length/2); i <= 1; i--) ...

which starts with floor(0/2) that is with i==0. That value is less than 1 so the loop makes the first iteration and decrements i with the i-- expression — and i becomes minus one. Then the loop proceeds until the i value wraps over zero to become positive (and greater than 1). For 32-bit integers it would take about 2.14 billion iterations...

The very first thing your main() does is invoking BuildHeap() which in turn invokes (multiple times) Heapify(). The latter compares array items to decide if they should get swapped – but they were not assigned any values! You never fill the A array with any data...

Did you notice that you compare and swap items which are outside your A array (actually, before it), since BuildHeap() decrements i below zero and passes negatve index values to Heapify()...?

Your Heapify routine performs at most one swap in the array. It doesn't seem sufficient. Suppose the heap is already 3 levels deep, and the current item appears the smallest one in the heap; it should go to the deepest level, but how do you achieve that? A single swap moves the item just one level downwards.


This is how you could do it.

First, decide the maximum size of data you will handle and prepare the array:

#define LENGTH 100

int A[LENGTH + 1];

(You need 'plus one' because A[0] will not be a part of the heap — why?)

Then prepare variables to store an actual size of data:

int dataSize;
int heapSize;

and a routine to fill the array. You can read the user input or load data from a file, or just fill the array by some algorithm.

void FillArrayAscending() {
    dataSize = 16;
    for(int i = 1; i <= dataSize; i ++)
        A[i] = i;
}
void FillArrayDescending() {
    dataSize = 25;
    for(int i = 1; i <= dataSize; i ++)
        A[i] = 50 - i;
}
void FillArrayCrossed() {
    dataSize = 20;
    for(int i = 1; i <= dataSize; i += 2)
        A[i] = 10 + i;
    for(int i = 2; i <= dataSize; i += 2)
        A[i] = 30 - i;
}

Then you can 'heapify'. To do that you sift the item down the heap – you iterate swapping until the item reaches its place:

void SiftDown(int i) {
    while(i < heapSize) {
        int largest = i;

        if(2*i <= heapSize && A[i] < A[2*i])
            largest = 2*i;
        if(2*i + 1 <= heapSize && A[largest] < A[2*i + 1])
            largest = 2*i + 1;

        if(largest == i)
            break;  // the item reached its place

        A[0] = A[largest];
        A[largest] = A[i];
        A[i] = A[0];
        i = largest;  // new position to sift down...
    }
}

i indicates the item to be sifted down, largest denotes a swap destination, A[0] can be used as a temporary storage (why?).

Now you can build a heap in the array:

void BuildHeap() {
    heapSize = dataSize;
    for(int i = heapSize/2; i >= 1; i --)
        SiftDown(i);
}

and retrieve items from a heap in a descending order:

void ExtractHeap() {
    while(heapSize > 1) {
        A[0] = A[1];        // take the max item from a heap
        A[1] = A[heapSize]; // replace it with the last one
        A[heapSize] = A[0]; // add the max one to the sorted part
        heapSize --;
        SiftDown(1);        // re-heapify
    }
}

Now you can do the whole processing:

int main(int, char**)
{
    FillArrayAscending();  // or Descending or...
    BuildHeap();
    ExtractHeap();

    // and here you can print the array contents
    // for indices 1 .. dataSize
    // to verify it is actually sorted
}
Sign up to request clarification or add additional context in comments.

1 Comment

I am very thankful for your comprehensive answer. It really help me to understand my problems. Thank you once again!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.