patterncppMinor
Inorder, Preorder, and Postorder traversal of a binary tree in C++
Viewed 0 times
postorderbinarypreorderandtreeinordertraversal
Problem
I am a newbie in this world of data structures and algorithms (returning to these topics after 5 long years). Can someone please check if the code I have written for Binary Tree is OK?
Test-case
10, 3,11,11,5,0
PrintInorder
3,0,5,10,11,11
PrintPreOrder
10,3,0,5,11,11
PrintPostOrder
3,0,5,11,11,10
```
using namespace std;
template
struct Node
{
T data;
Node* left;
Node* right;
Node()
{
left = nullptr;
right = nullptr;
}
};
template
class BTree
{
public:
BTree();
~BTree();
void Add(T val);
void Print(int o);
T Find(T key);
private:
Node* root;
Node* temp;
int height;
Node Add(T val, Node node);
void PrintInorder(Node* ptr);
void PrintPreOrder(Node* ptr);
void PrintPostOrder(Node* ptr);
};
template
T BTree::Find(T key)
{
Node* current = this->root;
while (current!= nullptr)
{
if (current->data == key)
{
return current->data;
}
else if(current->data > key)
{
current = current->left;
continue;
}
else if (current->data right;
continue;
}
}
}
template
void BTree::PrintInorder(Node* ptr)
{
PrintPreOrder(ptr->left);
coutdataright);
}
template
void BTree::PrintPostOrder(Node* ptr)
{
PrintPreOrder(ptr->left);
PrintPreOrder(ptr->right);
coutdata
BTree::BTree()
{
root = nullptr;
height = 0;
}
template
void BTree::PrintPreOrder(Node* ptr)
{
if (ptr!=nullptr)
{
coutdataleft!=nullptr)
PrintPreOrder(ptr->left);
if(ptr->right!=nullptr)
PrintPreOrder(ptr->right);
}
template
void BTree::Print(int o)
{
if (o==1)
{
PrintInorder(root);
}
else if (o==2)
{
PrintPreOrder(root);
}
else if(o==3)
{
PrintPostOrder(root);
}
}
template
BTree::~BTree()
{
}
template
void BTree:: Add(T val)
{
if (root==nullptr)
{
root = Add(val, nu
Test-case
10, 3,11,11,5,0
PrintInorder
3,0,5,10,11,11
PrintPreOrder
10,3,0,5,11,11
PrintPostOrder
3,0,5,11,11,10
```
using namespace std;
template
struct Node
{
T data;
Node* left;
Node* right;
Node()
{
left = nullptr;
right = nullptr;
}
};
template
class BTree
{
public:
BTree();
~BTree();
void Add(T val);
void Print(int o);
T Find(T key);
private:
Node* root;
Node* temp;
int height;
Node Add(T val, Node node);
void PrintInorder(Node* ptr);
void PrintPreOrder(Node* ptr);
void PrintPostOrder(Node* ptr);
};
template
T BTree::Find(T key)
{
Node* current = this->root;
while (current!= nullptr)
{
if (current->data == key)
{
return current->data;
}
else if(current->data > key)
{
current = current->left;
continue;
}
else if (current->data right;
continue;
}
}
}
template
void BTree::PrintInorder(Node* ptr)
{
PrintPreOrder(ptr->left);
coutdataright);
}
template
void BTree::PrintPostOrder(Node* ptr)
{
PrintPreOrder(ptr->left);
PrintPreOrder(ptr->right);
coutdata
BTree::BTree()
{
root = nullptr;
height = 0;
}
template
void BTree::PrintPreOrder(Node* ptr)
{
if (ptr!=nullptr)
{
coutdataleft!=nullptr)
PrintPreOrder(ptr->left);
if(ptr->right!=nullptr)
PrintPreOrder(ptr->right);
}
template
void BTree::Print(int o)
{
if (o==1)
{
PrintInorder(root);
}
else if (o==2)
{
PrintPreOrder(root);
}
else if(o==3)
{
PrintPostOrder(root);
}
}
template
BTree::~BTree()
{
}
template
void BTree:: Add(T val)
{
if (root==nullptr)
{
root = Add(val, nu
Solution
I can see a few changes that at least might be worth considering.
Minimize Requirements
All else being equal, you typically want to minimize the requirements you place on the type contained in a container like this. For example, where you need to do comparisons, you can use the same operator (typically `
Minimize Requirements
All else being equal, you typically want to minimize the requirements you place on the type contained in a container like this. For example, where you need to do comparisons, you can use the same operator (typically `
and ==).
Check recursion
Your PrintInOrder looks broken:
template
void BTree::PrintInorder(Node* ptr)
{
PrintPreOrder(ptr->left);
coutdataright);
}
This seems to do a pre-order traversal of the left sub-tree, prints the current, then a pre-order traversal of the right sub-tree. For in-order, you want an in-order traversal of the left-subtree, process the current node, then in-order traversal of the right subtree. With only a few nodes (so the tree is never very deep) this probably may not be visible, but it's clearly wrong.
The same applies to your PrintPostOrder.
Simplify where possible
At least to me, your PrintPreOrder seems unnecessarily complex. I'd do something like:
template
void PrintPreOrder(Node *ptr) {
if (ptr == nullptr)
return;
std::cout data left);
PrintPreOrder(ptr->right);
}
With this we only do one check for a null pointer in one place.
Likewise, your Add currently looks like this:
template
void BTree:: Add(T val)
{
if (root==nullptr)
{
root = Add(val, nullptr);
}
else
{
root = Add(val,root);
}
}
This checks whether root is a null pointer, and if it is it explicitly passes a null pointer. Otherwise, it pass root. If we just pass root, (whether it's a null pointer or not) we get exactly the same effect.
template
void BTree:: Add(T val)
{
Add(val, root);
}
Avoid std::endl
When people use std::endl, they typically just want to write a new-line to a stream. Unfortunately, that's not quite all that std::endl actually does--in addition to writing the new-line, it flushes the stream. Flushing the stream unnecessarily can lead to a dramatic speed loss (e.g., 10x is pretty common).
On the rare occasion that you actually do want to both write a new-line and flush the stream, I think it's better to do that explicitly:
std::cout << "\n" << std::flush;
At least in my experience, actual uses for this are fairly rare though.
Prefer enum's to magic numbers
Your Print currently sorts out the order for the printing like this:
if (o==1) // ...
else if (o==2) // ...
else if(o==3) // ...
Rather than 1, 2 and 3, I'd rather use an enumeration:
enum order { PRE_ORDER, IN_ORDER, POST_ORDER };
Then Print would look something like this:
template
void BTree::Print(order o)
{
switch (o) {
case PRE_ORDER: PrintPreOrder(root); break;
case IN_ORDER: PrintInOrder(root); break;
case POST_ORDER: PrintPostOrder(root); break;
}
}
At least to me, this seems considerably clearer and more readable (also note the use of a switch statement to directly express the intent of choosing one of multiple alternatives).
Don't Repeat Yourself
Right now, you have code in both Add and Find to find a node (if any) with a particular value. Oddly, the two use entirely different approaches to this problem. Rather than having two copies of code to find a node based on a key, I'd rather have one piece of code that's used from both:
Node *&find_key(T key) {
Node *current = root;
for (Node *current = root; current != nullptr;) {
if (key data)
current = current->left;
else if (current->data right;
else
break;
}
return current;
}
With this, we can rewrite Find to look something like:
T Find(T key) {
Node *pos = find_key(key);
if (pos != nullptr)
return pos->data;
}
Likewise, Add would look something like this:
void Add(T key) {
Node *&pos = find_key(key);
if (pos == nullptr)
pos = new Node(key);
}
Avoid Undefined Behavior
As currently defined, if you call Find for a key that isn't present in the tree, execution flows off the end of the function without returning a value, leading to undefined behavior. There are various ways to deal with this, such as a Maybe or Optional type that carries both the return value itself and an indication whether a value is actually present. Another possibility (probably not as nice, but at least it does work) is to use an approach about like the find_key` shown above, returning a null pointer if a value isn't present.Code Snippets
template<typename T>
T BTree<T>::Find(T key)
{
Node<T>* current = this->root;
while (current!= nullptr)
{
if (current->data < key)
current = current->right;
else if (key < current->data)
current = current->left;
else // Neither is less than the other, so they must be equal
return current->data;
}
}template<typename T>
void BTree<T>::PrintInorder(Node<T>* ptr)
{
PrintPreOrder(ptr->left);
cout<<ptr->data<<endl;
PrintPreOrder(ptr->right);
}template <class T>
void PrintPreOrder(Node<T> *ptr) {
if (ptr == nullptr)
return;
std::cout << ptr->data << "\n";
PrintPreOrder(ptr->left);
PrintPreOrder(ptr->right);
}template<typename T>
void BTree<T>:: Add(T val)
{
if (root==nullptr)
{
root = Add(val, nullptr);
}
else
{
root = Add(val,root);
}
}template<typename T>
void BTree<T>:: Add(T val)
{
Add(val, root);
}Context
StackExchange Code Review Q#128509, answer score: 8
Revisions (0)
No revisions yet.