Create a complete binary tree and do level order traversal on it How can I optimize this code?

#include&lt;iostream&gt;
#include&lt;queue&gt;
using namespace std;

struct Node
{
    int data;
    Node *left,*right;
};

Node *createNode(int data){
    Node *node=new Node;
    node-&gt;left=NULL;
    node-&gt;right=NULL;
    node-&gt;data=data;
    return node;
}

Node buildTree(Node *newNode,queue&lt;Node *&gt;&amp;q){
    Node *f=q.front();
    if(f-&gt;left &amp;&amp; f-&gt;right) {q.pop();f=q.front();}
    if(!f-&gt;left) f-&gt;left=newNode;
    else if(!f-&gt;right) f-&gt;right=newNode;
    q.push(newNode);
}


void levelOrder(Node *root){
    queue&lt;Node*&gt;q;
    if(root){
        q.push(root);
        while(!q.empty()){
            Node *d =q.front();
            q.pop();
            cout&lt;&lt;d-&gt;data&lt;&lt;" ";
            if(d-&gt;left) q.push(d-&gt;left);
            if(d-&gt;right) q.push(d-&gt;right);
        }
    }
}

int main(){
    Node *root=createNode(10);
    queue&lt;Node *&gt;q;
    q.push(root);
    for(int i=0;i&lt;10;i++){
        buildTree(createNode(i),q);
    }
    levelOrder(root);
    return 0;
}
using namespace std;


using namespace std;



Why is “using namespace std;” considered bad practice?

The particular problem I have with it here is that it obscures that you are using std::queue and not some custom class.  

Note that this can also cause problems if you later convert it to header file, which would be natural for a general use data structure.  

Don't go the long way around


Node *createNode(int data){
    Node *node=new Node;
    node-&gt;left=NULL;
    node-&gt;right=NULL;
    node-&gt;data=data;
    return node;
}



Rather than create a function for this, consider just creating a constructor for struct Node.  

Node::Node(int datum) {
    this-&gt;datum = datum;
    left = NULL;
    right = NULL;
}


Note that datum is the singular form of data, so I changed the field in Node to datum as it only holds one piece.  


        buildTree(createNode(i),q);



could become 

        buildTree(new Node(i), q);


This calls a constructor directly rather than creating a function just to call the default constructor and set the data.  

Why wait?


    if(f-&gt;left &amp;&amp; f-&gt;right) {q.pop();f=q.front();}
    if(!f-&gt;left) f-&gt;left=newNode;
    else if(!f-&gt;right) f-&gt;right=newNode;



This could be 

    if (f-&gt;left) {
        f-&gt;right = newNode;
        q.pop();
    } else {
        f-&gt;left = newNode;
    }


This works because you know that something is only in the queue until both its children are added.  Then it is removed.  So you don't need to check for the both children full case if you only ever add to the tree with this method.  

I changed the order because I find it easier to read positive conditions with else blocks.  An else is essentially the negation of the if.  This way we aren't negating a negative.  

What if something else adds to the tree?  That would be problematic.  Of course, that was already true.  Something else adding to the tree would interfere with the order that you use the queue to enforce.  

Removals are also problematic with either implementation.  

This is why Node would normally be a hidden data type, not exposed to callers.  Then you could be sure that only your tree class would be able to access it.  

Don't make data structures that you don't use


    queue&lt;Node*&gt;q;
    if(root){



If you do this in the other order, you can save the overhead of creating the std::queue.  

    if (root) {
        queue&lt;Node*&gt; q;


or even 

    if (!root) {
        return;
    }

    queue&lt;Node*&gt; q;


Which also reduces the indent.  

Efficiency

The biggest problem with efficiency that I see is that this doesn't seem to be a tree in any meaningful sense.  Sure, the data structure behind it is a tree, but the operations aren't.  You buildTree in insertion order.  Then you levelOrder to read back the tree in insertion order.  If you only wanted to do that, then a std::vector would be a much more efficient data structure.  You wouldn't need the std::queue to manage the insertions nor reading them back.  


    Node *root=createNode(10);
    queue&lt;Node *&gt;q;
    q.push(root);
    for(int i=0;i&lt;10;i++){
        buildTree(createNode(i),q);
    }
    levelOrder(root);



would become 

    std::vector&lt;int&gt; data;
    data.push_back(10);
    for (int i = 0; i &lt; 10; i++) {
        data.push_back(i);
    }

    for (auto datum : data) {
        std::cout &lt;&lt; datum &lt;&lt; " ";
    }


Isn't that simpler?  No new functions required.  Also, rather than using a pointer-based data structure and a queue, this just uses one array-based data structure.  You could also use a std::queue instead of a std::vector.  

Encapsulation

This is also encapsulated better.  Instead of having to create a tree structure and a queue in order to call buildTree, this simply calls push_back and lets std::vector handle the details internally.  Consider the following interface 

class InsertionOrderTree {
    struct Node {
        int datum;
        Node *left;
        Node *right;
    };

    Node *root;
    std::queue&lt;int&gt; q;

    public:
    InsertionOrderTree();
    build(int datum);
    traverse();
};

InsertionOrderTree::InsertionOrderTree() {
    root = NULL;
}


This hides the implementation detail of using a queue from any callers.  Callers shouldn't have to know how you do things, just what you promise to do.  They certainly should not have to create a specific data structure just to help you do whatever it is that you are doing.  You should create your own data structures if you need them.  

Note that the implementations of build and traverse would change from your original versions.  In particular, build needs to handle a NULL root.  

Unsolicited advice

This is a bad use case for a tree though.  You should probably either find a different use case that better fits a tree (if you just want to implement a tree) or use a different data structure (if this is really the problem that you need to solve).