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

Converting "a(b(cd)e(fg))" into a tree

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
convertingintotree

Problem

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
#include
#include

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->x=a[i];
            temp->left=temp->right=NULL;
            i++;
            if(p->left==NULL)
            {
                p->left=temp;
                tre(p->left);
            }
            if(a[i]!='('&&a[i]!=')')
            {
                struct node *tempp=malloc(sizeof(struct node));
                tempp->x=a[i];
                i++;
                p->right=tempp;
                tre(p->right);
            }
        }
        else
        if(a[i]==')')
        {
            i++;
        }

}    

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

main()
{
    printf("Enter the string : ");
    scanf("%s",a);
    struct node *temp=malloc(sizeof(struct node));
    temp->x=a[i];
    temp->left=temp->right=NULL;
    i++;
    root=temp;
    tre(root);
    inorder(root);
}

Solution

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->x=a[i];
            temp->left=temp->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->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->left=temp;
                tre(p->left);
            }
            if(a[i]!='('&&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->x=a[i];
                i++;
                p->right=tempp;
                tre(p->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->x = a[i++];
     if(a[i] == '(')
     {
          node->left = tre();
          node->right = tre();
          assert(a[i++] == ')');
     }
     else
     {
          node->left = node->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.

Code Snippets

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->x=a[i];
            temp->left=temp->right=NULL;
i++;
            if(p->left==NULL)
{
                p->left=temp;
                tre(p->left);
            }
            if(a[i]!='('&&a[i]!=')')
{
                struct node *tempp=malloc(sizeof(struct node));
                tempp->x=a[i];
                i++;
                p->right=tempp;
                tre(p->right);
            }

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

Context

StackExchange Code Review Q#6705, answer score: 2

Revisions (0)

No revisions yet.