0

On this implementation of Priority Queue I have to use recursion to determinate if the array that I receive as a parameter of the method IsMaxHeap is a max heap.

I want to be sure I evaluate all the cases that could make this work or not work without any problem.

static boolean isMaxHeap(int[] H, int idx) {

    if(2*idx == (H.length -1) && H[2 * idx] <= H[idx])
        return true;
    if(2*idx > (H.length -1))
        return true;
    if ((2*idx+1) == (H.length -1) && H[2 * idx] <= H[idx] && H[2 * idx + 1] <= H[idx])
        return true;
    if((2*idx+1) > (H.length -1))
        return true;

    if (H[2 * idx] <= H[idx] && H[2 * idx + 1] <= H[idx])
        return isMaxHeap(H, (idx + 1));
    else
        return false;

}

Could you help me?

CubeJockey
  • 2,209
  • 8
  • 24
  • 31
Lu MON
  • 107
  • 8
  • Surely your first `if` is going to result in a array out of bounds exception `H[2 * idx + 1]` – Scary Wombat Jan 07 '16 at 04:10
  • Just a suggestion: You might want to extract functions such as `getParent`, `getLeftChild`, `getRightChild` (that get the appropriate element using its index and known heap properties) from the rest of the code for readability and modularity. It could also point out repetitions more easily... – nem035 Jan 07 '16 at 04:56

1 Answers1

1

Your code is hard to follow because you do so many calculations in your conditionals. So it's hard to say whether it would actually work. Also, what you've written is basically a recursive implementation of a for loop. That is, you check nodes 1, 2, 3, 4, 5, etc.

Whereas that can work, you end up using O(n) stack space. If you have a very large heap (say, several hundred thousand items), you run the risk of overflowing the stack.

A more common way to implement this recursively is to do a depth-first traversal of the tree. That is, you follow the left child all the way to the root, then go up one level and check that node's right child, and its left children all the way to the root, etc. So, given this heap:

          1
    2           3
 4    5      6      7
8 9 10 11  12 13  14 15

You would check nodes in this order: [1, 2, 4, 8, 9, 5, 10, 11, 3, 6, 12, 13, 7, 14, 15]

Doing it that way simplifies your code and also limits your stack depth to O(log n), meaning that even with a million nodes your stack depth doesn't exceed 20.

Since you're using 2*idx and 2*idx+1 to find the children, I'm assuming that your array is set up so that your root node is at index 1. If the root is at index 0, then those calculations would be: 2*idx+1 and 2*idx+2.

static boolean isMaxHeap(int[] H, int idx)
{
    // Check for going off the end of the array
    if (idx >= H.length)
    {
        return true;
    }

    // Check the left child.
    int leftChild = 2*idx;
    if (leftChild < H.length)
    {
        if (H[leftChild] > H[idx])
           return false;
        if (!isMaxHeap(H, leftChild)
            return false;
    }

    // Check the right child.
    int rightChild = 2*idx + 1;
    if (rightChild < H.length)
    {
        if (H[rightChild] > H[idx])
            return false;
        return isMaxHeap(H, rightChild);
    }

    return true;
}
Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
  • Thank you ! You are right, but how i realize that my recursion method takes a lot of space of the stack frame? it is kind of hard! – Lu MON Jan 07 '16 at 17:43