2

I am very new to smart pointers and I am trying to create a doubly tree where the child nodes are pointed from the parents by a unique pointer, and the children are pointing to the parents via raw pointer. So when A parent node gets destroyed the whole sub-tree will get destroyed in the process.

class Node {
private:
    Node *parent;
    std::unique_ptr<Node> left;
    std::unique_ptr<Node> right;
public:
    Node(Node* _left, Node* _right, Node* _parent);
};

Node::Node(Node* _left, Node* _right, Node* _parent) {
    parent = &_parent;

    //this is where the problem starts
}

I don't understand how to point to a new node that might have a tree I want to connect. If I use make_unique I believe that will create a new node Instead of preserving the tree.

I might be totally wrong about this since I just learned smart pointers about 4 days ago (Realistically enough time to learn something).

  • 1
    The things you are passing to the constructor are copies of nodes, which is unlikely to be what you want. –  Sep 07 '17 at 20:05
  • Arguments passed by values are no different from other local variables, and will go out of scope once the function returns. – Some programmer dude Sep 07 '17 at 20:06
  • How can left and right be in the constructor? You are creating a node while already knowing the parent and the children? – Chiel Sep 07 '17 at 20:11
  • well As I said in the explanation, I want the unique properties of the unique pointer class(where if it goes out of scope it gets deleted), but at the same time want access to its parents. – Dogunbound hounds Sep 07 '17 at 20:18

3 Answers3

5

First of all, an empty tree is possible and a default constructed node will fit well. Parent reference will be known at the time a node is attached so, child's node parent shall be updated once a node is set as left or right child of the current tree.

It might be a good idea to receive unique_ptr as you are taking ownership of the pointer you receive. Here is an example implementation:

class Node {
private:
  Node *parent = nullptr;
  std::unique_ptr<Node> m_left;
  std::unique_ptr<Node> m_right;

public:

  void left(std::unique_ptr<Node> child) {
    m_left = std::move(child);
    m_left->parent = this;
  }

  void right(std::unique_ptr<Node> child) {
    m_right = std::move(child);
    m_right->parent = this;
  }
};

You will use it like the following:

int main()
{
  auto tree = std::make_unique<Node>();
  auto subtree = std::make_unique<Node>();

  subtree->right(std::make_unique<Node>());
  tree->right(std::make_unique<Node>());
  tree->left(std::move(subtree));
  return 0;
}

I'm pretty new to unique_ptr myself, hope someone will further correct me. BTW don't use names hat that starts with _ for your identifies, they are reserved.

  • 1
    "TW don't use names hat starts with _ for your identifies, they are reserved." Sometimes. [What are the rules about using an underscore in a C++ identifier?](https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) I don't use them because I don't want an surprises if I accidentally violate one of the rules. – user4581301 Sep 07 '17 at 21:01
  • Well, that was my spirit too. – Giuseppe Puoti Sep 07 '17 at 21:07
2

I don't think you can use:

Node(Node _left, Node _right, Node _parent);

This won't allow to build the tree node by node. Instead, use:

Node(Node* _left, Node* _right, Node* _parent);

That way, you can create the first node using:

Node firstNode(nullptr, nullptr, nullptr);

From there, you can build other nodes.

To build a simple tree, with three nodes as below

            N1
           /  \
         N2    N3

you can use:

Node* N1 = new Node(nullptr, nullptr, nullptr);
Node* N2 = new Node(nullptr, nullptr, N1);
Node* N3 = new Node(nullptr, nullptr, N1);

N1->left = N2;  // Use the equivalent member function.
N1->right = N3;
R Sahu
  • 196,807
  • 13
  • 136
  • 247
  • 2
    Better to use `std::unique_ptr` or even `std::make_unique()` when constructing `N1`, `N2` and `N3`, and then `std::move()` `N2` and `N3` into `N1->left` and `N1->right`. Also, the `_left` and `_right` constructor parameters should be `std::unique_ptr` as well – Remy Lebeau Sep 07 '17 at 20:38
  • Hey how did you build that tree? I might try and remember how and provide better questions this way. – Dogunbound hounds Sep 07 '17 at 21:19
  • @Dogunboundhounds, you mean the tree in ASCII art? – R Sahu Sep 07 '17 at 21:20
  • @RSahu Yes. How do I do that? In stackOverflow editor. – Dogunbound hounds Sep 07 '17 at 21:34
  • @Dogunboundhounds, just like writing code. Use the characters `/` and `\ ` to create the connections. It's a bit tedious if you are not used to it but not so bad after you get used to it. – R Sahu Sep 07 '17 at 21:36
  • @RSahu so you then put it in a "code block" to make it stand out. Wow now I feel real stupid. – Dogunbound hounds Sep 07 '17 at 21:41
  • @RemyLebeau, My compiler, g++ 5.4.0, does not support `std::make_unique`. Can't try the code before I post it. Don't want to post something that potentially has compiler errors. – R Sahu Sep 07 '17 at 21:42
  • @RSahu There are online compilers. – Dogunbound hounds Sep 07 '17 at 21:43
  • @Dogunboundhounds, no need to feel stupid. We call can learn from the tricks used by others around us. – R Sahu Sep 07 '17 at 21:43
  • @RSahu you can still declare `N1`, `N2` and `N3` as `std::unique_ptr` without having `std::make_unique()`, just use `new` directly, eg: `std::unique_ptr N1(new Node(nullptr, nullptr, nullptr)); ... ` – Remy Lebeau Sep 07 '17 at 22:49
0

I believe that you want to make the parent, left and right child public. At least, this is how I have always implemented my nodes using a struct instead:

struct Node
{
    Node(std::unique_ptr<Node> _parent = nullptr, 
std::unique_ptr<Node> _left = nullptr, std::unique_ptr<Node> _right = nullptr) : parent(_parent), left(_left), right(_right) {}

    std::unique_ptr<Node> parent;
    std::unique_ptr<Node> left;
    std::unique_ptr<Node> right;

};

Someone please correct me if I am wrong.

Sailanarmo
  • 996
  • 8
  • 29
  • 2
    By necessity one `Node`'s `parent` is going to be another `Node`'s `left` or `right`. Not a very unique pointer if there are two pointers to it. OP has this part right. – user4581301 Sep 07 '17 at 20:23
  • @user4581301 ahh that makes sense. I have never really used `std::unique_ptr`, I have always used `std::shared_ptr` and wrongly assumed that unique and shared could be used in the same way. – Sailanarmo Sep 07 '17 at 20:24
  • Good answer describing what the various smart pointers are for in plain english: https://stackoverflow.com/a/30143936/4581301 . The first answer to the question goes into better detail on how and when to use. – user4581301 Sep 07 '17 at 20:29
  • @user4581301 thank you for the link. My professor showed me how to build a node using `std::shared_ptr`, so if we replaced all the unique with shared, would that be bad practice? Or is that acceptable if it gets the job done? – Sailanarmo Sep 07 '17 at 20:33
  • https://www.youtube.com/watch?v=JfmTagWcqoE&t=1864s Go to 22:41. This is why I posted this question – Dogunbound hounds Sep 07 '17 at 20:44
  • @Dogunboundhounds thanks for the link. Bookmarked. Sailanarmo, I make the `shared_ptr` call on a case-by-case basis. Sometimes it doesn't make sense for `Node`s to manage themselves because they don't know enough about the rest of the container to make the call (like cutting loose a node from a graph and not noticing the Node is part of a self-referencing cycle) a graph sitting lost in memory.. Sometimes the overhead of the `shared_ptr` hinders performance (there's more than just a simple reference counter variable in there). – user4581301 Sep 07 '17 at 20:58