1

I am writing a c++ implementation of a BST tree using std::unique_ptr.

I am a hobbyist programmer. Initially, I wrote an insert function using a helper function and passing the pointer using move semantics, but that forced me to return unique_ptr. I then considered passing a pointer to the unique_ptr. I even considered using unique_ptr::get() and sending a pointer to the binaryNode, but I have read that this function should only be used when interfacing with legacy functions (Implementation not shown). 

template<class K, class V = char>
struct binaryNode
{
    using nodePtr = std::unique_ptr<binaryNode<K, V>>;
    using keyType = K;
    using valueType = V;

    binaryNode(keyType key, valueType val) : _key(key), _value(val) {};
    binaryNode(const binaryNode& n) = delete;
    ~binaryNode() = default;

    keyType _key = keyType();
    valueType _value = valueType();
    nodePtr l_node = nullptr;
    nodePtr r_node = nullptr;
};

template<class K, class V = char>
class BSTree
{
public:
    using nodePtr = std::unique_ptr<binaryNode<K, V>>;
    using keyType = K;
    using valueType = V;

    BSTree() {}
    void insert(const keyType & key, const valueType & value);
    void insert2(const keyType & key, const valueType & value);

private:
    nodePtr insertHelper(nodePtr && root, const K & key, const V & value);
    void insertHelper2(nodePtr * root, const K & key, const V & value);

    nodePtr _root = nullptr;
};

template<class K, class V>
void BSTree<K, V>::insert(const keyType & key)
{
    _root = insertHelper(std::move(_root), key);
    if (!isBalanced(_root)) _root = rebalance(std::move(_root));
}

template<class K, class V>
typename BSTree<K, V>::nodePtr BSTree<K, V>::insertHelper(nodePtr && root, const keyType & key, const valueType & value)
{
    if (root == nullptr) return std::make_unique<binaryNode<K, V>>(std::move(binaryNode<K, V>(key)));

    if (key < root->_key) root->l_node = insertHelper(std::move(root->l_node), key, value);
    if (key > root->_key) root->r_node = insertHelper(std::move(root->r_node), key, value);

    return std::move(root);
}

template<class K, class V>
void BSTree<K, V>::insert2(const keyType & key, const valueType & value)
{
    insertHelper2(&_root, key, value);
    if (!isBalanced(_root)) _root = rebalance(std::move(_root));
}

template<class K, class V>
void BSTree<K, V>::insertHelper2(nodePtr * root, const K & key, const V & value)
{
    if (*root == nullptr) *root = std::make_unique<binaryNode<K, V>>(std::move(binaryNode<K, V>(key, value)));

    if (key < (*root)->_key) insertHelper(&((*root)->l_node), key, value);
    if (key > (*root)->_key) insertHelper(&((*root)->r_node), key, value);
}

Functionally, these two approaches give the same tree structures. I haven't tried timing the two approaches, but I am curious as to which of these approaches is considered "correct"? Or is there a better approach of which I have not thought?

EDIT: typos fixed

davidbear
  • 375
  • 2
  • 13
  • 4
    Let me ask you something else. Who owns a node? The parent node or the tree? – StoryTeller - Unslander Monica Jan 06 '19 at 20:40
  • I believe it is the parent node with the exception of the root node which is owned by the tree. I am not sure as to how to have the tree own all of the nodes – davidbear Jan 06 '19 at 20:43
  • 2
    Well, one usually encapsulates the node entirely in the tree. Then the ownership becomes much clearer. Then there's only one entity that can control the lifetime of any node in a given tree, it's the tree itself. – StoryTeller - Unslander Monica Jan 06 '19 at 20:46
  • 1
    Could you give an example as to how to do that or point me to a website that might help me figure out what you are saying? I think you are suggesting that the tree have a container of unique_ptr to nodes and that the nodes have left and right pointers to those stored unique_ptrs. But I'm not sure that I see the advantage to that approach. – davidbear Jan 06 '19 at 21:04
  • 2
    I'm actually suggesting you dispense with the unique_ptr. If you think about it, it will start making sense, and things will become simpler. The tree *structure* is how you handle ownership here. – StoryTeller - Unslander Monica Jan 06 '19 at 21:56
  • 3
    Why do `insertHelper` and `insertHelper2` need to take `root` as a parameter at all? They are member functions `BSTree` - they can just access `_root` data member. It's also unclear why they need to return anything. Or why a helper is needed in the first place. – Igor Tandetnik Jan 06 '19 at 22:00
  • 1
    @StoryTeller am I correct that you do not agree with this answer: https://stackoverflow.com/a/24591763/7782896 or am I misunderstanding your previous comment? – davidbear Jan 06 '19 at 22:00
  • 2
    That's correct. A unique_ptr is not just a "self deleting pointer". It expresses ownership, and in this case I'd say it's not the ownership one wants. For instance, a node being rotated doesn't stop belonging to the tree. So why would one need to keep realeasing and reacquiring it? The unique_ptr becomes an artificial construct here. – StoryTeller - Unslander Monica Jan 06 '19 at 22:05
  • @IgorTandetnik The helper functions are acting recursively, so they need a node upon which to act. insertHelper passes the left node to itself and then acts on the left and right nodes of that node. or am I misunderstanding your point? – davidbear Jan 06 '19 at 22:09
  • @StoryTeller thank you for the clarification. So, would you use a container (say vector) of unique pointers as part of the tree structure? – davidbear Jan 06 '19 at 22:12
  • In this case, I'd probably hand them a raw `binaryNode*` pointer via `get`, as you are not transferring ownership to the helper function. And I'd call the parameter something like `currentNode`: `root` seems misleading. – Igor Tandetnik Jan 06 '19 at 22:13
  • @IgorTandetnik fair enough poor name choice. So, you don't like the idea of passing a pointer to the unique_ptr (insertHelper2)? – davidbear Jan 06 '19 at 22:18
  • `nodePtr* root` suggests to me that you plan to modify `root` - e.g. call `reset` to make it point to some other node. Do you? – Igor Tandetnik Jan 06 '19 at 22:20
  • @IgorTandetnik OK I think I get it. I might want to do some modification when I go to rebalance the tree, but for that I'll likely move the unique_ptr itself. – davidbear Jan 06 '19 at 22:30

0 Answers0