HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Recursive binary search tree

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
recursivebinarytreesearch

Problem

How is my usage of std::unique_ptr() and std::move()? How can I improve this code?

```
//I don't know what to return when key is not found, so throw std::runtime_error()
//this code does not have handwritten copy/move/destruct because I think unique_ptr already handled the node pointers
template
using Ptr = std::unique_ptr;

template
class Bst {

struct Node {
K key;
V val;
std::size_t n;
Ptr left;
Ptr right;

Node(const K& k, const V& v, const std::size_t& s) : key(k), val(v), n(s), left(nullptr), right(nullptr) { }
};

Ptr root;

public:

Bst() :root{nullptr} { }

std::size_t size() {
return size(root);
}

std::size_t size(Ptr& t) {
return (t == nullptr) ? 0 : t->n;
}

void insert(const K& k, const V& v) {
root = insert(root,k,v);
}

Ptr insert(Ptr& t, const K& k, const V& v) {
if (t == nullptr) {
Ptr node(new Node(k,v,1));
return node;
}
if (k key) {
t->left = insert(t->left,k,v);
} else if (k > t->key) {
t->right = insert(t->right,k,v);
} else {
t->val = v;
}
t->n = size(t->left)+size(t->right)+1;
return std::move(t);
}

V& operator[](const K& k) {
return get(root,k);
}

V& get(Ptr& t, const K& k) {
if (t == nullptr) {
t = insert(t,k,{});
return t->val;
}
if (k key) {
return get(t->left,k);
} else if (k > t->key) {
return get(t->right,k);
} else {
return t->val;
}
}

K min() {
check(root);
return min(root)->key;;
}

Ptr& min(Ptr& t) {
if (t->left == nullptr) {
return t;
}
return min(t->left);
}

K max() {
check(root);
return max(root)->key;
}

Ptr& max(Ptr& t) {
if (t->right == nullptr)

Solution

Please don't introduce global name aliases in a header file like this:

template
using Ptr = std::unique_ptr;


You're setting yourself up for name collisions and other nasties. Just use the god... I mean committee given names.

If you really want a shorthand, then use a type-local alias. Like this:

template
class Bst {
private:
    struct Node;
public:
    typedef std::unique_ptr NodePtr;


It encapsulates the shorthand with the context in which it is used. As an added benefit you can now change the implementation from unique_ptr to shared_ptr if need be, without affecting the rest of the application.

The C++11 keyword auto will save your clients typing Bst::NodePtr. Also NodePtr is shorter to type than Ptr ;)

I agree with RubberDuck on the naming. Prefer BinarySearchTree as class name. Also I'm leaving the indepth review for some one with more time on their hands.

Code Snippets

template<typename T>
using Ptr = std::unique_ptr<T>;
template<typename K, typename V>
class Bst {
private:
    struct Node;
public:
    typedef std::unique_ptr<Node> NodePtr;

Context

StackExchange Code Review Q#76898, answer score: 7

Revisions (0)

No revisions yet.