3

Ok I have tried to make the Hauffman Code by my own and I have a problem when I'm trying to print the corresponding code to each letter with a recursive method.

(By this point I already created a BinaryTree and root is not NULL) Each node has a value and if it's a leaf it also has a letter, so I begin going down the tree node by node but it seems that in the recursive method the node just forgot who he was or I don't know, I have tried a lot of things. :S

The recursive method is createCode(Node* actual, string actualCode);

Here's the code:

#include <iostream>
#include <fstream>
#include <stdio.h>
#include <string>
#include <queue>
using namespace std;

#define MAX_VALUE 256
int frequencies[MAX_VALUE] = {0};

struct Node {
    int value;
    char letter;
    struct Node *Left, *Right;
}*root=NULL, *temp, *left_temp, *right_temp;

struct CompareNode : public std::binary_function<Node*, Node*, bool> {
    bool operator()(const Node* lhs, const Node* rhs) const {
        if (lhs->value == rhs->value)
            return lhs->letter > rhs->letter;
        else
            return lhs->value > rhs->value;
    }
};

void createCode(Node* actual,string actualCode) {
    if (actual->Left == NULL && actual->Right == NULL) {
        cout << "For: " << actual->letter << " is " << actualCode << endl;
    }
    else {
        if (actual->Left) {
            createCode(actual->Left, actualCode + "0");
        }
        if (actual->Right) {
            createCode(actual->Right, actualCode + "1");
        }
    }
}

void createTree() {
    priority_queue<Node*, vector<Node*>, CompareNode> que;

    for (int x = 0; x < MAX_VALUE; x++) {
        if (frequencies[x] > 0) {
            temp = new Node;
            temp->value = frequencies[x];
            temp->letter = char(x);
            que.push(temp);
        }
    }

    while (que.size() > 1) {
        temp = new Node();
        temp->Left = que.top();
        que.pop();
        temp->Right = que.top();
        que.pop();
        temp->value = temp->Left->value + temp->Right->value;
        temp->letter = NULL;
        que.push(temp);
    }

    root = que.top();
    que.pop();
}

void fillArray(int argc, char *argv[]) {
    string line;
    const char* ptr;

    ifstream myFile(argv[argc - 1]);
    if (myFile.is_open()) {
        while (myFile.good()) {
            getline(myFile,line);

            if (line.length() != 0) {
                ptr = &line.at(0);

                while (*ptr != '\0')
                    ++frequencies[*ptr++];
            }
        }
    }
    else
        cout << "The file could not be open.";
}

int main(int argc, char *argv[]) {
    fillArray(argc, argv);

    createTree();

    createCode(root, "");
    return 0;
}

This is the example tree (I tried to post the image but because I'm new I could't):

Binary Tree Image

And here's the output:

For: a is 0
For:   is 10
Segmentation fault: 11

Please help :(

  • Most likely, `actual->Left` or `actual->Right` point to a `Node` that is corrupt or non-existent. You can use tools like `valgrind` to track the problem. (Perhaps you allocate a `Node` and didn't set `Left` and `Right` to NULL?) – David Schwartz Nov 15 '12 at 01:45
  • 2
    If a node has only one child (which _should_ not happen in a Huffmann tree), `createChild` will crash trying to access the other child as well. – John Dvorak Nov 15 '12 at 01:45
  • 2
    @JanDvorak: Good catch. The giveaway would be that in that case, the crash would occur in `createCode` and `actual` would be `NULL`. – David Schwartz Nov 15 '12 at 01:46
  • @DavidSchwartz too bad we don't have the stack trace. – John Dvorak Nov 15 '12 at 01:47
  • What am I missing here guys? Looks like `root` is set to 0 and then used before ever being initialized (and that assignment would result in an error anyway) – Ed S. Nov 15 '12 at 01:48
  • @EdS. not shown in the code: `root` is modified before being walked. – John Dvorak Nov 15 '12 at 01:49
  • @JanDvorak: That would be an important piece of info. I was just going to post an answer describing how this code wouldn't even compile and, if `root` were intended to be a pointer, the cause would be obvious. The example introduces another possible segfault. – Ed S. Nov 15 '12 at 01:50
  • @EdS. `(By this point I already created a BinaryTree and root is not NULL)` – John Dvorak Nov 15 '12 at 01:51
  • @JanDvorak: Right, but as the OP is unable to deduce the cause of the problem I am wary of taking that statement at face value. Often people *think* they are doing the correct thing, but of course they're not, which is why they came here. The example should not omit details which may be relevant. Also, again, that code wouldn't even compile. – Ed S. Nov 15 '12 at 01:52
  • @EdS. I can write a "the bug is in your other code" answer. – John Dvorak Nov 15 '12 at 01:54
  • @JanDvorak: not sure what that means, but ok. Point is, don't introduce additional bugs in your example and certainly do not post code which doesn't compile (unless you are asking how to solve a compiler error of course). – Ed S. Nov 15 '12 at 01:55
  • Ok I edit the code and now that is the complete code. I already check "if each child exist then proceed to check it" but the error is the same. – Jordan Cortes Guzman Nov 15 '12 at 02:04
  • `int frequencies[MAX_VALUE] = {0};` IIUC this will produce an array with its first element equal to zero. Try `int frequencies[MAX_VALUE]()` – John Dvorak Nov 15 '12 at 02:09

1 Answers1

1

You need to initialise the Left and Right pointers to NULL when creating the Nodes you push into the queue:

if (frequencies[x] > 0) {
    temp = new Node;
    temp->Left = NULL;
    temp->Right = NULL;
    temp->value = frequencies[x];
    temp->letter = char(x);
    que.push(temp);
}

Without explicit initialisation, these pointers have indeterminate values, and often not NULL, thus in createCode you are dereferencing invalid pointers.

Daniel Fischer
  • 181,706
  • 17
  • 308
  • 431