Why is this code correct while it should clearly run into an infinite loop?

Question

I have been having a problem with this code for a while. The placement of recursive call of the function does not seem right.

i tried running the code and yes it does run into a infinite loop.

// I DEFINE HEAP STRUCTURE AS :
struct heap_array
{
  int *array;  // heap implementation using arrays(note : heap is atype of a tree).

  int capacity;  // how much the heap can hold.
  int size;   //how much size is currently occupied.

void MaxHeapify(struct heap_array *h,int loc)  // note : loc is the location of element to be PERCOLATED DOWN.
{
  int left,right,max_loc=loc;
  left=left_loc_child(h,loc);
  right=right_loc_child(h,loc);

  if(left !=-1 && h->array[left]>h->array[loc])
  {
    max_loc=left;
  }

  if(right!=-1 && h->array[right]>h->array[max_loc])
  {
    max_loc=right;
  }

  if(max_loc!=loc)  //i.e. if changes were made:
  {
    //swap the element at max_loc and loc
    int temp=h->array[max_loc];
    h->array[max_loc]=h->array[loc];
    h->array[loc]=temp;


  }
    MaxHeapify(h,max_loc); // <-- i feel that this recursive call is misplaced. I have seen the exact same code in almost all the online videos and some books i referred to. ALSO I THINK THAT THE CALL SHOULD BE MADE WITHIN THE SCOPE OF condition if(max_loc!=loc).
    //if no changes made, end the func right there.
}

score 0 · Accepted Answer · answered Jul 09 '19 at 14:08

0

In your current implementation, it looks like you don't have a base case for recursion to stop.

Remember that you need a base case in a recursive function (in this case, your MaxHeapify function), and it doesn't look like there is one.

Here is an example of MaxHeap which may be resourceful to look at

// A recursive function to max heapify the given 
    // subtree. This function assumes that the left and 
    // right subtrees are already heapified, we only need 
    // to fix the root. 
    private void maxHeapify(int pos) 
    { 
        if (isLeaf(pos)) 
            return; 

        if (Heap[pos] < Heap[leftChild(pos)] ||  
            Heap[pos] < Heap[rightChild(pos)]) { 

            if (Heap[leftChild(pos)] > Heap[rightChild(pos)]) { 
                swap(pos, leftChild(pos)); 
                maxHeapify(leftChild(pos)); 
            } 
            else { 
                swap(pos, rightChild(pos)); 
                maxHeapify(rightChild(pos)); 
            } 
        } 
    }

Here, you can see the basecase of:

    if (isLeaf(pos)) 
        return;

You need to add a base case to your recursive function.

answered Jul 09 '19 at 14:08

artemis

6,857
11
46
99

I understand what u meant. I DO need a base condition, but the above code is not mine but the code popularly shown in many books and online lessons while it is wrong. About the base condition , wont it be correct if just call the heapify function if any changes were made that is the condt. if(max_loc!=loc) is TRUE ?? – chaitanya_12789 Jul 09 '19 at 14:14
I had another doubt, the above code(assume it is correct) wont heapify the entire heap ,right?, as there might still be some index which does not follow MAX heap condt, – chaitanya_12789 Jul 09 '19 at 14:16
Can you please edit your question to show where that code is located? I don't remember seeing it anywhere I have looked. And as for your questions, the above code would NOT heapify the entire heap in its current state, but I believe because that is a code segment. If you click on the GeeksforGeeks link I posted, that shows a complete example that would. Additionally, just that conditional would not satisfy a base case, as you are still editing the heap in every possible condition, so there is no return statement. – artemis Jul 09 '19 at 14:21
Thanks! i saw the code in a book called DS made easy by narasimha , on youtube channel like Saurabh Shukla and some more. I hve only changed the names , rest is same. :) – chaitanya_12789 Jul 09 '19 at 14:52

Why is this code correct while it should clearly run into an infinite loop?

1 Answers1