Popping/deleting nodes from a Huffman Tree Minheap

Question

I'm having trouble popping correctly from a Huffman Tree. Right now I am creating a Huffman Tree based off a minheap and I want to do the following:

If we assume A and B to be two different subtrees, I would say that A would be popped off first if A's frequency is less than B's frequency. If they have the same frequency, then I would find the smallest character in ASCII value in any of A's leaf nodes. Then I would see if that smallest character leaf node in A is smaller than that in any of B's leaf nodes. If so I would pop off A before B. If not I would pop off B. <- this is what I'm having trouble with.

For example:

Let's assume I input:

eeffgghh\n (every letter except for \n's frequency which is 1 is 2)

into my Huffman Tree. Then my tree would look like this:

        9      
       / \
    5        4    
   / \      / \
  3  h      g  f
 /\
e  \n

Below is my attempt for popping out of my Huffman minheap. I am having trouble with the part of comparing if the frequencies of two letters are the same. If anyone could help, that would be great. Thanks!

void minHeap::heapDown(int index)
{
  HuffmanNode *t = new HuffmanNode();
  if(arr[index]->getFreq() == arr[left]->getFreq() || arr[index]->getFreq() == arr[right]->getFreq()) //arr is an array of HeapNodes
    {
      if(arr[left]->getLetter() < arr[right]->getLetter())
      {
        t = arr[index]; //equals operator is overloaded for swapping
        arr[index] = arr[left];
        arr[left] = t;
        heapDown(left);
      }
      else
      {
        t = arr[index];
        arr[index] = arr[right];
        arr[right] = t;
        heapDown(right);
      }
    }
    if(arr[index]->getFreq() > arr[left]->getFreq() || arr[index]->getFreq() > arr[right]->getFreq())
    {
      if(arr[left]->getFreq() < arr[right]->getFreq())
      {
        t = arr[index];
        arr[index] = arr[left];
        arr[left] = t;
        heapDown(left);
      }
      else
      {
        t = arr[index];
        arr[index] = arr[right];
        arr[right]  = t;
        heapDown(right);
      }//else
    }//if
}

score 1 · Answer 1 · answered Dec 01 '12 at 05:43

The standard C++ library contains heap algorithms. Unless you're not allowed to use it, you might well find it easier.

The standard C++ library also contains swap(a, b), which would be a lot more readable than the swap you're doing. However, swapping in heapDown is inefficient: what you should do is hold onto the element to be placed in a temporary, then sift children down until you find a place to put the element, and then put it there.

Your code would also be a lot more readable if you implemented operator< for HuffmanNode. In any event, you're doing one more comparison than is necessary; what you really want to do is (leaving out lots of details):

heapDown(int index, Node* value) {
  int left = 2 * min - 1;  // (where do you do this in your code???)
  // It's not this simple because you have to check if left and right both exist
  min = *array[left] < *array[left + 1] ? left : left + 1;  // 1 comparison
  if (array[min] < to_place) {
    array[index] = array[min];
    heapDown(min, value);
  } else {
     array[index] = value;
  }

Your first comparison (third line) is completely wrong. a == b || a == c does not imply that b==c, or indeed give you any information about which of b and c is less. Doing only the second comparison on b and c will usually give you the wrong answer.

Finally, you're doing a new unnecessarily on every invocation, but never doing a delete. So you are slowly but inexorably leaking memory.

Popping/deleting nodes from a Huffman Tree Minheap

1 Answers1