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

BST C++ STL implementation, visiting algorithms

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

Problem

I am currently learning C++, and as an exercise I tried to implement a BST in a C++11. I am not sure at all if what I have done could be considered a good example of c++ programming, thus I would love to know from you if you believe my code could be improved in terms of functionalities, efficiency, or just programming style.

In particular, these are my doubts:

  • Is the use I made of unique_ptr a good idea in this situation? (It should be because of RAII, from what I got.)



  • Is the use of nested class a good idea in this situation, or even in general? In this case I think that maybe it could not be a problem because all its members are public but the class itself is encapsulated within the BinarySearchTree, so the inner class is not visible from outside the BinarySearchTreeclass.



  • I am not really sure about the checkBST function. Could it be better?



I had a look at others implementations that I was able to find on the web, in order to correct my first (and I must admit too much C-like first attempt).

```
template class BinarySearchTree {
private:
// nested class for the node of the tree
class TreeNode {
public:
std::unique_ptr left;
std::unique_ptr right;
T value;
TreeNode(T val) {
value = val;
}
};
// private attributes / methods of BST class
std::unique_ptr root; // root of the BST
int V; // number of nodes
void private_recursive_inorderVisit(TreeNode *node) {
if(node == nullptr)
return;

private_inorderVisit(node->left.get());
std::cout value right.get());
}

void private_iterative_inorderVisit(TreeNode *node) {
TreeNode *currNode = node;
std::stack s;
while(s.size() > 0 || currNode != nullptr) {
if(currNode != nullptr) {
s.push(currNode);
currNode = currNode->left.get();
} else {
currNode = s.top();

Solution

Okay some minor things here:

As I mentioned in a comment:

Naming

Why did you give your private methods the prefix private_ ? Looks code noise to me. Also these methods use cout. You should consider using an ostream-parameter (that may, or may not be defaulted to cout) instead. This way you could let the caller of the method decide whether he wants the output in a console, or a file, or sent to the international space station.

One of your methods is called inorderVisit. Consider calling it visitInOrder as method names should generally start with a verb.

Pointers

Next thing:

std::unique_ptr node(new TreeNode(RootValue));


This line of code obviously works, but generally it is recommended to use make_unique instead:

std::unique_ptr node = std::make_unique(RootValue);


Also you are mixing unique_ptr with raw pointers. Consider using weak_ptr or shared_ptr where necessary.

Nested class

I rarely ever use nested classes, but your case here seems like it's worth an exception. This inner class is only used inside the outer class and has no sense whatsoever outside of the class. So making it an inner class seems like the way to go.

Code Snippets

std::unique_ptr<TreeNode> node(new TreeNode(RootValue));
std::unique_ptr<TreeNode> node = std::make_unique<TreeNode>(RootValue);

Context

StackExchange Code Review Q#100855, answer score: 2

Revisions (0)

No revisions yet.