patterncppModerate
Preorder, inorder, postorder for tree in C++11
Viewed 0 times
inorderforpreordertreepostorder
Problem
Please review my code for preorder, inorder and postorder of a tree. I am using C++11 (or wanted to use), so let me know if anything is deviating from the C++11 standard.
#include
#include
using std::cout;
using std::endl;
using std::shared_ptr;
struct node
{
int data;
shared_ptr left;
shared_ptr right;
};
shared_ptr newNode (int i)
{
shared_ptr n (new node);
n->data = i;
n->left = nullptr;
n->right = nullptr;
return n;
}
void preOrder(shared_ptr n)
{
if(n == nullptr)
return;
coutdataleft));
preOrder((n->right));
}
void inOrder(shared_ptr n)
{
if(n == nullptr)
return;
inOrder((n->left));
coutdataright));
}
void postOrder(shared_ptr n)
{
if(n == nullptr)
return;
postOrder((n->left));
postOrder((n->right));
coutdata r = newNode(1);
(r->left) = newNode(2);
(r->right) = newNode(3);
(r->left->left) = newNode(4);
(r->left->right) = newNode(5);
preOrder(r);
cout<<endl;
inOrder(r);
cout<<endl;
postOrder(r);
cout<<endl;
return 0;
}Solution
Memory management
Don't think that is the appropriate smart pointer.
Do you have plans to actually share ownership of a node? Or is it more likely that ownership will be done by the patent of the node? I would have gone with
make_X
You should not be using
Using
I suppose this is better than
But its still pretty sloppy and lazy. Pull those declarations into the tightest scope possible (ie inside a function).
Braces
This looks strange
Now I am all for using braces to make expression easy to read or to force a particular evaluation order. But this just looks strange and does not help in the evaluation.
Prefer '\n'
Prefer '\n' to std::endl.
The difference between the two is that
Don't think that is the appropriate smart pointer.
shared_ptr newNode (int i)Do you have plans to actually share ownership of a node? Or is it more likely that ownership will be done by the patent of the node? I would have gone with
unique_ptr if I was going to use smart pointers.make_X
You should not be using
new with smart pointers. There are make_XXX commands that combine all the memory allocation required into a single operation (thus making it more efficient).std::shared_ptr n = std::make_shared(val); // or make_uniqueUsing
I suppose this is better than
using namespace std;using std::cout;
using std::endl;
using std::shared_ptr;But its still pretty sloppy and lazy. Pull those declarations into the tightest scope possible (ie inside a function).
Braces
This looks strange
preOrder((n->left));
preOrder((n->right));Now I am all for using braces to make expression easy to read or to force a particular evaluation order. But this just looks strange and does not help in the evaluation.
Prefer '\n'
Prefer '\n' to std::endl.
The difference between the two is that
std::endl forces a flush after placing the new line on the stream. It is very rare that you actually want to force a flush (as the runtime is much better at making that decision than you). Excessive flushing just makes the stream libraries slow.- “\n” or '\n' or std::endl to std::cout?
- Why does endl get used as a synonym for “\n” even though it incurs significant performance penalties?
Code Snippets
shared_ptr<node> newNode (int i)std::shared_ptr<node> n = std::make_shared<node>(val); // or make_uniqueusing std::cout;
using std::endl;
using std::shared_ptr;preOrder((n->left));
preOrder((n->right));Context
StackExchange Code Review Q#77304, answer score: 10
Revisions (0)
No revisions yet.