Creating a smart pointer in a struct?

Question

I'm modeling a Node in a Binary Tree using a struct. In the struct, I'm trying to have a pointer to the left and right child.

The problem is, I keep running into a stack overflow due to the way I'm creating the struct. It seems the way I've been handling the smart pointers continuously allocates memory on the stack.

The exception is specifically thrown when I create theroot in my main.

I'm new to smart pointers (I've been using raw pointers which I have recently learned is bad practice in C++), and I have tried solving this issue on my own without luck.

Can someone critique my struct/smart pointer use? Many thanks.

#include <iostream> 
#include <memory> 

//Node struct 
struct Node
{
    int data;
    std::unique_ptr<Node> left;
    std::unique_ptr<Node> right;

    Node(int data) {
        this->data = data;
        this->left = std::make_unique<Node>(NULL);
        this->right = std::make_unique<Node>(NULL); 
    }

};

//insert Node into binary search tree
void insert(int data, std::unique_ptr<Node>& root)
{
    if (root == NULL)
    {
        root = std::make_unique<Node>(data);
    }
    else {
        if (root->data > data)
        {
            insert(data, root->left);
        }
        else {
            insert(data, root->right);
        }
    }
}

//In Order tree traversal 
void inOrderTraversal(std::unique_ptr<Node>& root)
{
    if (root == NULL) return; 

    inOrderTraversal(root->left); 

    std::cout << root->data << std::endl; 

    inOrderTraversal(root->right); 
}

int main()
{
    //Initialize root to NULL
    std::unique_ptr<Node> root = std::make_unique<Node>(NULL);


    insert(20, root); 
    insert(50, root);
    insert(30, root);
    insert(5, root);
    insert(6, root);
    insert(99, root);
    insert(77, root);
    insert(56, root);
    insert(32, root);
    inOrderTraversal(root); 

    return 0; 
}

Kostas · Accepted Answer · 2018-08-26T02:28:05.503

The function std::make_unique<Node> takes parameters to forward the Node constructor.

In C and C++ NULL is usually just a macro for 0.

Therefore, when you call std::make_unique<Node>(NULL); you are initializing a Node, using data = 0.

This then recursively calls this->left = std::make_unique<Node>(NULL);, causing infinite recursion and stack overflow eventually.

To solve this you can assign std::unique_ptr<Node> left = NULL.

I would also suggest using nullptr in the place of NULL because it is type safe. Simply replacing NULL with nullptr on your code gives compiler errors, helping you resolve the issue.

error: no matching constructor for initialization of 'Node'

score 1 · Answer 2 · answered Aug 26 '18 at 02:13

1

Replace all NULL with nullptr and don't use std::make_unique(NULL);

Node::Node(int data) {
    this->data = data;
    this->left = nullptr;
    this->right = nullptr;
}



int main()
{
    //Initialize root to NULL
    std::unique_ptr<Node> root = nullptr;

        // other codes ..
}

answered Aug 26 '18 at 02:13

seccpur

4,996
2
13
21

Initializing with `nullptr` is no different than the default constructor, so `this->left` and `this->right` don't need to be in the constructor body at all. And `root` can be created with `std::unique_ptr root;` – Kevin Aug 26 '18 at 02:22

Creating a smart pointer in a struct?

2 Answers2