Converting "a(b(cd)e(fg))" into a tree This was actually asked in an interview. I have made it in the best possible way I could and would like to optimize it if there is any chance of doing it.

The input is:

a(b(cd)e(fg)) 


The output should be:      

                            a
                           / \
                          b   e
                         / \ / \
                        c  d f  g


My current code is:

#include&lt;stdio.h&gt;
#include&lt;malloc.h&gt;
#include&lt;string.h&gt;

struct node
{
    char x;
    struct node *left;
    struct node *right;
}*root;

char a[30];
int i=0,n;

void tre(struct node * p) //function that I am using to convert the string to a tree
{
        if(a[i]=='(')
        {
            i++;
            struct node *temp=malloc(sizeof(struct node));
            temp-&gt;x=a[i];
            temp-&gt;left=temp-&gt;right=NULL;
            i++;
            if(p-&gt;left==NULL)
            {
                p-&gt;left=temp;
                tre(p-&gt;left);
            }
            if(a[i]!='('&amp;&amp;a[i]!=')')
            {
                struct node *tempp=malloc(sizeof(struct node));
                tempp-&gt;x=a[i];
                i++;
                p-&gt;right=tempp;
                tre(p-&gt;right);
            }
        }
        else
        if(a[i]==')')
        {
            i++;
        }

}    

void inorder(struct node *p)//inorder traversal of the tree made
{    
    if(p!=NULL)
    {
        inorder(p-&gt;left);
        printf("%c ",p-&gt;x);
        inorder(p-&gt;right);
    }
}

main()
{
    printf("Enter the string : ");
    scanf("%s",a);
    struct node *temp=malloc(sizeof(struct node));
    temp-&gt;x=a[i];
    temp-&gt;left=temp-&gt;right=NULL;
    i++;
    root=temp;
    tre(root);
    inorder(root);
}
void tre(struct node * p) //function that I am using to convert the string to a tree
{
        if(a[i]=='(')
        {


I recommend not using such short variable names

            i++;
            struct node *temp=malloc(sizeof(struct node));
            temp-&gt;x=a[i];
            temp-&gt;left=temp-&gt;right=NULL;


You basically repeat these three lines three times. If you instead do this at the beginning of tre you can avoid the duplication.

            i++;
            if(p-&gt;left==NULL)


Given the way your code is structured, won't p->left always be null? The value passed to tre is always a freshly created node

            {
                p-&gt;left=temp;
                tre(p-&gt;left);
            }
            if(a[i]!='('&amp;&amp;a[i]!=')')


Why do you need to check for this? Wouldn't this indicate an invalid input? If so, perhaps you should report the error somehow.

            {
                struct node *tempp=malloc(sizeof(struct node));
                tempp-&gt;x=a[i];
                i++;
                p-&gt;right=tempp;
                tre(p-&gt;right);
            }

        }
        else
        if(a[i]==')')
        {
            i++;
        }


Again, in what situation will this be called? 

}    


How I'd approach that function

stuct node * tre()
{ 
     struct node * node = malloc(sizeof(struct node));
     node-&gt;x = a[i++];
     if(a[i] == '(')
     {
          node-&gt;left = tre();
          node-&gt;right = tre();
          assert(a[i++] == ')');
     }
     else
     {
          node-&gt;left = node-&gt;right = NULL;
     }
     return node;
}


Unless I'm missing something, your example output and code do not match. So I'm not going to comment on that.