I'm working on a function that inserts a value in a binary search tree, i wrote this code:

// this is my decalarations if its relevent
typedef struct element* tree;

typedef struct element
{  
    type data;
    tree right;
    tree left;
}noed;

tree rightSon(tree head)
{
    return (head->right);
}

void insert(tree* a,int val)
{
      
     if(!empty(*a))
     {
         if((*a)->data>val)
         {    
             if(!empty(leftSon(*a)))
                  insert(&leftSon(*a),val); // error here leftson return a tree
            else 
            {
             (*a)->left=newNoed(val);
            
            }
             
         }
         else if((*a)->data<val)
         {
              if(!empty(rightSon(*a)))
                  insert(&rightSon(*a),val); //same error here
            else 
             (*a)->right=newNoed(val);
         }
         else printf("value already exist!\n");
    }
    else
    {
        *a=newNoed(val);
    }
}

i solved the problem by declaring local variables of type tree then assigning the values to them like this:

tree lson;
lson=leftson(*a);
insert(&lson,val);

But i still don't get what was the problem in the first place.

CodePudding user response：

It seems the function leftSon (and rightSon) has the return type tree. That is it returns a temporary object. You may not apply the operator & to a temporary object like in this statement

insert(&leftSon(*a),val);

You could write the code without calling the functions leftSon or rightSon like

 if(!empty(rightSon(*a)))
     insert( &(*a)->right ,val);

In fact the if-else statements like this

         if(!empty(leftSon(*a)))
              insert(&leftSon(*a),val); // error here leftson return a tree
        else 
        {
         (*a)->left=newNoed(val);
        
        }

are redundant. Instead of them you could just write

insert( &(*a)->left, val );

and

insert( &(*a)->right, val );

CodePudding user response：

Your rightSon function is returning the value of the pointer object stored at head->right. You can't take the address of a value.

What you want to do instead is have rightSon return the address of head->right which can then be passed directly to insert.

So change rightSon to return the address:

tree *rightSon(tree head)
{
    return &head->right;
}

And call it like this:

insert(rightSon(*a),val); 

leftSon presumably has the same issue, so make a similar change for that.

Also, hiding a pointer behind a typedef is considered bad practice as it's no longer obvious by just looking at the code that a variable of that type is a pointer and can cause confusion to the reader.