0

I have a problem with this recursive void function:

void ReadBinaryTree(NODE &p)
{
    char ch;
    File.get(ch);
    if (ch != '#')
    {
        NODE p = new Node;
        p->SetFreq(ch);
        ReadBinaryTree(p->Left());
        ReadBinaryTree(p->Right());
    }
    else return;
}

NODE is a pointer to the class Node and File is an ifstream object. Left() and Right() return NODE. The compiler does not accept the p->Left() and p->Right() arguments, the error is:

no know conversion for argument 1 from 'NODE {aka Node*} to 'Node *&

Could you help me? How can I save the pointers to Node during the recursion?

Improve this question
7
  • 5
    Ugh, don't hide pointers behind typedefs! Commented Dec 28, 2013 at 13:18
  • @LaszloPapp: It's almost certainly a source of confusion that's making life difficult for the OP here. Pointers and value types have different syntax and semantics; there's very rarely a case where disguising one as the other is helpful. Commented Dec 28, 2013 at 13:23
  • Please construct a minimal test-case that we can try to compile ourselves (say, in ideone.com). Commented Dec 28, 2013 at 13:24
  • @OliCharlesworth: you really cannot say it ultimately unless you are a style purist. Commented Dec 28, 2013 at 13:25
  • @LaszloPapp: I didn't say "ultimately" ;) What I am saying, though, is that one shouldn't do it unless there's a very good reason to. It's not a question of style. Commented Dec 28, 2013 at 13:26

3 Answers 3

Reset to default
2

Make it so that Left and Right return references. You are trying to pass temporary values into ReadBinaryTree, which don't bind to the non-const reference in the argument list.

Your recursion is broken anyway, since inside the function you're operating on a local NODE p, not the argument. Maybe you meant p = new Node;, though it's hard to tell from this.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

The issue is that p->Left() and p->Right() are r-values and this means you cannot make references of them. The problem arises because p->Left() is returning some value (Of type Node *), which is being copied into the parameter of ReadBinaryTree.

To fix this, your p->Left() and p->Right() functions should return NODE & values (So that your recursion has access to the exact same pointer as is in the parent node, and not just some copy of it).

Also, NODE p = new Node; should just be p = new Node;.

However, can I suggest that you convert this function into one that returns a value? i.e.

NODE ReadBinaryTree(ifstream &File)
{
    char ch; File.get(ch);
    if(ch != '#')
    {
        NODE p = new Node;
        p->setFreq(ch);
        p->Left() = ReadBinaryTree(File);
        p->Right() = ReadBinaryTree(File);

        return p;
    } else return null;
}

This code still needs the left and right accessors to return references.

EDIT: Explanation of why references can't take r-values

p->Left() (the accessor function) is returning a copy of the left pointer. So while the value is the same as p->left (the attribute), the location in memory is different, and importantly, p->Left() has been assigned a temporary memory location, because it is an expression, and it was expecting a variable to be 'put into'. Normally, when you pass the expression into a function, it is 'put into' the parameter it is passed in as, but if that parameter is a reference, it expects there to already be a non-temporary memory location, which it adopts as its own. So, when you pass an expression (which has a temporary memory location) to something expecting a reference, it complains because it was expecting something with an already fixed memory location, for it to adopt.

Improve this answer

5 Comments

Oh thank you so much! There were 2 serious logical errors! I changed my code as you said and now it works but i don't understand why i need to return a reference to the pointer where the address of the child is stored. Why when i call the function i pass a simple pointer to Node but in the recursion i need a reference to the pointer?
Because initially, the left and right pointers are null. Your function needs to modify the pointer value so that it points to the new node you constructed. This means you need to have access to the reference of the pointer, and not just a copy of the pointer. Because changing a copy of the pointer in the function would still leave the parent's child nodes pointing to null.
Yes I know, but when the argument is the pointer and the parameter has the "&" operand you are automatically passing the reference to the pointer. Is it true? So I thought I had only to pass the pointer to the child because the parameter is the reference.
Ah right, yes, I've added an explanation to the end of my answer that hopefully answers your question.
Awesome! Thank you! cout << Root->Left()->GetKey(); prints the key of the left child but Left() returns a reference to the pointer! How is it possible? It works in the same way as before! I think it jumps from an address to another and it's more difficult for the compiler!
0

ReadBinaryTree needs a reference to a pointer, so that it can update the pointer to point to the newly created object. The problem is that Left() and Right() only returns the address to the child nodes, but not a reference to the pointer where the address is stored.

So you could change the signature of Left() and Right() from:

NODE Left()

to:

NODE & Left()

As a alternative, you could directly pass the pointers where the child nodes are stored, to ReadBinaryTree:

ReadBinaryTree (p->left)

(This naturally assumes there is a member in Node called left)

Improve this answer

1 Comment

Why when i call the function i pass a simple pointer to Node but in the recursion i need a reference to the pointer?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.