0

I am trying to create a binary heap with arrays. I have succesfully made the heap with buildHeap and heapify. My problem is when I try to insert a new element into the array, and when I try to sort it with heapSort.

Here is what I have for the heapify function:

void heap::Heapify(int arr[], int i){
    int largest = i;
    int L = LeftChild(i);
    int R = RightChild(i);
    if (L <= heapSize && arr[L] > arr[i])
        largest = L;
    else{
        largest = i;
    }

    if (R <= heapSize && arr[R] > arr[largest]){
        largest = R;
    }
    if(largest != i){
        int temp = arr[i];
        arr[i] = arr[largest];
        arr[largest] = temp;
        Heapify(arr, largest);
    }
}

And here is my buildHeap function:

void heap::BuildHeap(int arr[]){
    for(int i = (heapSize/2)-1; i >= 0; i--){
        Heapify(arr, i);
    }
}

These two functions work, while the following are not working, for the insert it is not inserting it in the proper location.

Here is the insert function:

void heap::Insert(int arr[], int key){
    heapSize++;
    int i = heapSize - 1;

    while(i > 0 && arr[Parent(i)] <= key){
        arr[i] = arr[Parent(i)];
        i = Parent(i);
    }

    arr[i] = key;
}

With the heapSort function it is sorting it for the most part but will print it like so (the first line is the heap before it is sorted):

32 24 5 19 23 4 3 11 2 12 
5 2 4 3 23 19 32 11 24 12 

and here is my heapSort function:

void heap::HeapSort(int arr[]){
    for(int i = heapSize - 1; i >= 0; i--){
        int temp = arr[0];
        arr[0] = arr[i];
        arr[i] = temp;

        heapSize = heapSize - 1;
        Heapify(arr, 0);
    }
}

Any help to figure how these two functions are not working properly would be greatly appreciated.

zsloan112
  • 43
  • 2
  • 9
  • 1
    looks like a simple debugging task. If you try to swap two values, but after the swap both are the same and the other value was lost, that would be bad. For a proper [mcve] I would expect to copy-paste into my compiler and get the same problem you got. – Kenny Ostrom Oct 10 '17 at 15:49
  • You really should review the theory behind the binary heap. The sequence 32 24 5 19 23 4 3 11 2 12 does not look like a valid content. Heap sort does not sort in place the internal data of the heap but extracts min/max value one by one. – jszpilewski Oct 10 '17 at 15:50
  • `arr[i] = arr[Parent(i)];` should perhaps be `std::swap(arr[i], arr[Parent(i)])` – AndyG Oct 10 '17 at 15:50

1 Answers1

0

I think the problem is Off-by-One-Error in these lines

if (L <= heapSize && arr[L] > arr[i])

if (R <= heapSize && arr[R] > arr[largest]){

they should both be < instead of <= because the last element is in heapsSize-1.

Other details

void heap::Heapify(int arr[], int i){
    int largest = i; // setting largest to i first time
    int L = LeftChild(i);
    int R = RightChild(i);
    if (L <= heapSize && arr[L] > arr[i]) // compare with arr[i] is not wrong but doesn't express the intent.
        largest = L;
    else{
        largest = i; // setting largest to i second time
    }

    if (R <= heapSize && arr[R] > arr[largest]){
        largest = R;
    }
    if(largest != i){
        int temp = arr[i];      // these 3 lines are the std::swap
        arr[i] = arr[largest];  // or you could roll your own function that does the same.
        arr[largest] = temp;    // better expressing the intent.
        Heapify(arr, largest);
    }
}

Rewritten to

void heap::Heapify(int arr[], int i){
    int largest = i;
    int L = LeftChild(i);
    int R = RightChild(i);

    if (L < heapSize && arr[L] > arr[largest]) {
        largest = L;
     }

    if (R < heapSize && arr[R] > arr[largest]) {
        largest = R;
    }

    if(largest != i){
        swap(arr[i], arr[largest]);
        Heapify(arr, largest);
    }
}
Surt
  • 15,501
  • 3
  • 23
  • 39