Data Structure -- C++ pointer pointing to itself

Question

I am new to C++. Originally a java developer

I created a binarytree in C++. It added methods to add values and prints values .

But somewhere I have done some mistake and a pointer is pointing to itself in the printNode() method

I am pasting my code below. Can anyone help me in figuring out the mistake I made

main.cpp

#include <iostream>
#include <newbinarytree.cpp>

using namespace std;

int main()
{

    NewBinaryTree* newtree;
    NewBinaryTree tree;
    newtree = &tree;
    newtree->add(10);
    newtree->add(11);
    newtree->add(7); **-->ALL goes well til here & debugger shows a proper tree**
    newtree->printTree();**--> This method does not print the tree values properly**

    return 0;
}

newbinarytree.cpp

#include <iostream>

using namespace std;

class Node{
public:
    int data;
    Node* left =NULL;
    Node* right =NULL;

    Node(int d){
        data = d;

    }
};

    class NewBinaryTree{
    private:
        Node* root = NULL;

    public:
        NewBinaryTree(){
            cout<<"Inside NewBinaryTree constructor";
        }

        void add(int dt){

            if(root ==NULL){
                Node node(dt);
                root = &node;
            }else{
                addAsChildOf(root,dt);
            }
        }

        void addAsChildOf(Node* tnode,int dt){
            if(dt < tnode->data){
                if(tnode->left == NULL){
                    Node newnode = Node(dt);
                    tnode->left = &newnode;
                }else{
                    addAsChildOf(tnode->left,dt);
                }
            }else{
                if(tnode->right == NULL){
                    Node newnode = Node(dt);
                    tnode->right = &newnode;
                }else{
                    addAsChildOf(tnode->right,dt);
                }
            }
        }

        void printTree(){
            if(root==NULL){
                cout<<"empty tree";
            }else{
                printNode(this->root);
            }
        }

        void printNode(Node* node1){ **--> This method goes into infinite loop**
            if(node1==NULL){
                return;
            }else{
                cout<<node1->data<<"[";

                if(node1->left != NULL){
                    cout<<"Left:";
                    printNode(node1->left);
                }
                if(node1->right != NULL){
                    cout<<"Right:";
                    printNode(node1->right);
                }
                cout<<"]";
            }
        }

    };

My Console output

10[Left:4210843[]Right:2686564[Left:2686564
[Left:2686564[Left:2686564[Left:2686564[Left:2686564[Left:2686564[Left:2686564[Left:.... an so on

Yes I tried, the left pointer in printNode() method is pointing to itself in 2nd recursion.. but I am not able to figure out where did I go wrong.. — Nostalgic, Feb 28 '19 at 04:52

TrebledJ · Accepted Answer · 2019-02-28T05:00:42.730

Your add methods are faulty in newbinarytree.cpp. You're creating a temporary Node object which gets destructed automatically as the scope exits. This results in undefined behaviour, explaining why you're getting bogus values from your printTree method. (I think there is usually a warning that notifies you of creating temporary objects.) Here's how I'd modify your add method.

void add(int dt){
    if (root == NULL) {
        Node *node = new Node(dt);
        root = node;
    } else {
       addAsChildOf(root,dt);
    }
}

You should do the same with your addAsChildOf method as it uses the same, incorrect way of constructing and adding a new node.

To gain a better understanding of why your code doesn't work:

if (root == NULL) {
    Node node(dt);  //  construct Node object. All is well.
    root = &node;   //  set Node object. All is well.
}   //  end of scope. Node is destructed. Not well.

Additionally, please use nullptr when setting pointers to a null pointer. Remember to also delete your dynamically-allocated Nodes to prevent memory leaks.

Hope this helps.

Ok Thanks.. In fact I do have another code which used to create node the way you did above and of course i loved it because this is how we do in java.. but i read comments that we need to delete it explicitly otherwise it might result in memory leaks.. also someone advised that new should be avoided in real time applications.. but as you pointed node will get destructed on scope exit — Nostalgic, Feb 28 '19 at 04:58
@RohitGaneshan "_i read comments that we need to delete it explicitly otherwise it might result in memory leaks_" Yes, you need to handle that with `new`. "_also someone advised that new should be avoided in real time application_" Right, generally `unique_ptr` and `shared_ptr` are recommended over raw `new` and `delete`, but often `new` is used in practice when implementing trees or lists. Do look into `unique_ptr` and `shared_ptr`. — TrebledJ, Feb 28 '19 at 05:02

Data Structure -- C++ pointer pointing to itself

main.cpp

newbinarytree.cpp

My Console output

1 Answers1