patterncppMinor
Binary tree with C++11 smart pointers
Viewed 0 times
withsmartbinarypointerstree
Problem
I'm trying to replace the use of raw pointers with smart pointers in my C++ code. The following bit is from my first attempt at a self-balancing binary tree, though there is nothing self-balancing at the moment. I am worried about some fundamental problems regarding the use of unique pointers.
template
class rbtree {
struct rbnode;
typedef std::unique_ptr node_ptr;
node_ptr root_;
struct rbnode {
node_ptr left, right;
rbnode *parent;
enum { Red, Black } color;
D data;
K key;
rbnode():
left(nullptr), right(nullptr), parent(nullptr),
color(Red), data(0), key(0) {}
rbnode(K k, D d):
left(nullptr), right(nullptr), parent(nullptr),
color(Red), data(d), key(k) {}
rbnode(K k, D d, rbnode *p):
left(nullptr), right(nullptr), parent(p),
color(Red), data(d), key(k) {}
};
node_ptr insert_(node_ptr& node, node_ptr& parent, K& key, D& data);
public:
bool insert(K& key, D& data);
void dfs(std::function visitor);
};
template
void rbtree::dfs(std::function visitor) {
}
template
typename rbtree::node_ptr rbtree::insert_(
node_ptr& node, node_ptr& parent, K& key, D& data) {
if (node == 0) { node = node_ptr(new rbnode(key, data, parent.get())); }
if (key == node->key) { throw "Key exists!"; }
if (key key) {
node->left = std::move(insert_(node->left, node, key, data));
} else {
node->right = std::move(insert_(node->right, node, key, data));
}
return std::move(node);
}
template
bool rbtree::insert(K& key, D& data) {
root_ = std::move(insert_(root_, root_, key, data));
root_->color = rbnode::Black;
return true;
}Solution
Unless I missed something, it appears that
Also, I propose a different approach than Loki's one for the default constructors of
This way, your default constructor can be reduced to its simplest form:
And the "default" values from the in-class member initializers will also be used to automatically initialize non-initialized members in the other constructors:
Note that I also used list initialization for some of the in-class members initializers and for the constructor initialization list. List initialization can help to avoid some problems related to implicit narrowing conversions of values.
bool rbtree::insert(K& key, D& data) always returns true. It is quite misleading. If there is no way for this function to ever return anything but true, you could as well get rid of the bool return type.Also, I propose a different approach than Loki's one for the default constructors of
rbnode: you can use in-class members initializers for the default values:struct rbnode
{
node_ptr left { nullptr };
node_ptr right { nullptr };
rbnode *parent { nullptr };
enum { Red, Black } color = Red;
D data;
K key;
// Etc...
};This way, your default constructor can be reduced to its simplest form:
rbnode() = default;And the "default" values from the in-class member initializers will also be used to automatically initialize non-initialized members in the other constructors:
rbnode(K k, D d):
data{d},
key{k}
{}
rbnode(K k, D d, rbnode* p):
parent{p},
data{d},
key{k}
{}Note that I also used list initialization for some of the in-class members initializers and for the constructor initialization list. List initialization can help to avoid some problems related to implicit narrowing conversions of values.
Code Snippets
struct rbnode
{
node_ptr left { nullptr };
node_ptr right { nullptr };
rbnode *parent { nullptr };
enum { Red, Black } color = Red;
D data;
K key;
// Etc...
};rbnode() = default;rbnode(K k, D d):
data{d},
key{k}
{}
rbnode(K k, D d, rbnode* p):
parent{p},
data{d},
key{k}
{}Context
StackExchange Code Review Q#51111, answer score: 6
Revisions (0)
No revisions yet.