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

Preorder, inorder, postorder for tree in C++11

Submitted by: @import:stackexchange-codereview··
0
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.

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_unique


Using

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_unique
using 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.