CPP:How to get the REAL element in a vector?

Question

I'm new of C++ and I'm making a huffman tree written in c++, and I'm in trouble in generating tree structure. Here is my code:

    void Huffman::generateTree() {
        std::vector<Node> nodes;
        for (auto itr : freq) {
            nodes.push_back(*(new Node(itr.first, itr.second)));
        }
        while (nodes.size() > 1) {
            Node::sort(&nodes);
            Node leftNode = nodes.back();
            nodes.pop_back();
            Node rightNode = nodes.back();
            nodes.pop_back();
            Node newAnode = *new Node();
            newAnode.merge(&leftNode,&rightNode);
            nodes.push_back(newAnode);
        }
        root = &nodes.back();
        displayTree();
    }

It finally combined to one node, but the left node and right node is wrong: (For debug purpose, every node have a random ID when created so that I found problem.)

NODE freq26|id96
|-Left:
   NODE freq10|id17
   |-Left:
   |--ERROR:SELF freq10|id17
   |-Right:
      NODE freq16|id19
      |-Left:
         NODE freq10|id17
         |-Left:
         |--ERROR:SELF freq10|id17
         |-Right:
            NODE freq16|id19
            |-Left:
               NODE freq10|id17
               |-Left:
               |--ERROR:SELF freq10|id17
               |-Right:
                  NODE freq16|id19
...endless loop

Just like the output, start from the first subnode, every nodes' left child node is it self and right node is another node but only two switching each other, and finally looped.

I searched and read several post about this and I know it maybe caused by pop_back and push_back elements to vector, the pointer may be point to another element, but how I can resolved it? Is there any way to get the REAL element in vector that not affected by operating vector?

Here are some code of my Node:

//header
    class Node {
    private:
        char value;
        int frequency;
        Node *left;
        Node *right;
        String code;
...
    class Huffman{
    private:
        Node * root;
        std::map<char,int> freq;// calculated frequency of data
        bool generated;
//source
    bool Node::sortMethod::operator()(const Node &nodeA, const Node &nodeB) {
        return nodeA.getFrequency() > nodeB.getFrequency();//getFrequency(): simply return node's frequency int
    }
    void Node::sort(std::vector<Node> *nodes) {
        std::sort(nodes->begin(), nodes->end(), sortMethod());
    }
    Node *Node::merge(Node *pLeft, Node *pRight) {
        left = pLeft;
        right = pRight;
        frequency = left->getFrequency() + right->getFrequency();
        return this;
    }

Everything helpful is welcome, thanks!

Answer 1

As the comments suggest, you need to stop thinking that C++ is similar to Java, it really isn't.

Objects in C++ have explicit lifetimes, and the language doesn't stop you from holding onto a reference or pointer to an object after it has ceased to exist.

If you want something to outlive the call it was created in, it needs to be dynamically allocated, and something should track it's lifetime. The default is std::unique_ptr, which deletes what it points to when the pointer is destroyed. You can transfer ownership by moving the unique_ptr.

Unfortunately, you can't usefully have a std::priority_queue of std::unique_ptr as you can't move the top, and pop doesn't return the value. Otherwise I would suggest using that rather than re-implementing it.

I don't think you have to sort your nodes each loop, as the merged node will always have the greatest frequency, so they go at the back.

class Node;
// Aliases for types we will use
using NodePtr = std::unique_ptr<Node>;
using Nodes = std::vector<NodePtr>;

class Node {
private:
    char value;
    int frequency;
    NodePtr left;
    NodePtr right;
    std::string code;

    Node(char value, int frequency) : value(value), frequency(frequency) /*, code?*/{}
    Node(NodePtr left, NodePtr right) : /*value? ,*/ frequency(left->frequency + right->frequency), left(std::move(left)), right(std::move(right)) /*, code?*/{}
... 
};

// N.b. not members of Node
bool compareNodePtrs(const NodePtr & left, const NodePtr & right) {
    return left->getFrequency() > right->getFrequency();
}

void sortNodes(Nodes & nodes) {
    std::sort(nodes.begin(), nodes.end(), compareNodePtrs);
}

void Huffman::generateTree() {
    Nodes nodes;
    for (auto itr : freq) {
        nodes.emplace_back(std::make_unique<Node>(itr.first, itr.second));
    }
    sortNodes(nodes);
    while (nodes.size() > 1) {
        auto left = std::move(nodes.back());
        nodes.pop_back();
        auto right = std::move(nodes.back());
        nodes.pop_back();            
        nodes.emplace_back(std::move(left), std::move(right));
    }
    root = std::move(nodes.back());
    displayTree();
}