Functions with pointers

Question

Just when I thought I had pointers down, it looks like I'm still a bit confused. I'm writing the operator= overload, so I want to deallocate memory then assign new. I'm dealing with a Quad tree class, where each tree has a pointer to root node and a root node has 4 pointers to 4 children, and those each have 4 children. So the operator= should just make a copy of the other tree's root and return it. So after deallocating and such, I come to wanting to allocate new memory and assigning. So I do:

root=new QtreeNode;
root=nodeCopier(*(source.root));

And here's my nodeCopier signature:

QNode nodeCopier(const QtreeNode & n) {
    QtreeNode tempNode;

    //stuff

     return tempNode;
}

But then I get this error error:

no matching function for call to
    Qtree::nodeCopier(Qtree::QtreeNode* const&)
    qtree.h:92: note: candidates are: Qtree::QtreeNode Quadtree::nodeCopier(const Qtree::QtreeNode&)

How do I fix this?

Could you post your declaration of the class `QtreeNode`? – Null Set Apr 01 '11 at 17:28 — Null Set, Apr 01 '11 at 17:28

asveikau · Accepted Answer · 2011-04-01T18:32:29.497

Taking away the arguments of nodeCopier, this part doesn't look right to me..

root=new QtreeNode;
root=nodeCopier( /* ... */ );

nodeCopier returns QNode (which based on your return statement is implicitly castable from QtreeNode) however from the first line we can it is being assigned to a QtreeNode*. Or put more generally you're assinging a non-pointer quantity into a pointer.

It looks like you might have wanted to do:

*root = nodeCopier( /* ... */ );

A second problem I can see based on the second line here and the error message.

root=nodeCopier(*(source.root));

Qtree::nodeCopier(Qtree::QtreeNode* const&)
qtree.h:92: note: candidates are:
   Qtree::QtreeNode Quadtree::nodeCopier(const Qtree::QtreeNode&)

Based on this it looks like source.root is a QtreeNode**, since you dereferenced it with * and that expression evidently yielded QtreeNode*. Either that or root does some pretty funky operator overloading. At any rate you passed a QTreeNode* into a function expecting QTreeNode&; you should use **source.root or (better yet) re-evaluate if source.root needs to be of type QtreeNode**. (My guess is it does not.)

Edit: I agree with what others are saying that the idiomatic C++ way is to create a copy constructor. However I guess my approach has been to help explain why it doesn't compile. I guess to help you bridge the conceptual gap it would be good to get some practice with more C-style pointer manipulation...

Eamonn McEvoy · Answer 2 · 2011-04-01T17:33:42.297

0

you dont need to return a QtreeNode if you passing in a pointer instead you should

root=new QtreeNode();                       //declare a new QtreeNode
root=nodeCopier(&(source.root));            //pass a pointer to root into nodeCopier

void nodeCopier(const QtreeNode* n) {       
    QtreeNode tempNode = new QtreeNode();   //declare a local QtreeNode

    //stuff

   *n = tempNode;                           //dereference root pointer and assign tempnode
   delete(tempNode);                        //Delete tempNode to prevent memory Leak
}

Hope this helps,

Eamonn

edited Apr 01 '11 at 17:33

answered Apr 01 '11 at 17:22

Eamonn McEvoy

8,876
14
53
83

I'm still getting an error here. `n` is not a pointer, so why are you dereferencing it in the last line? – Snowman Apr 01 '11 at 17:31
Sorry, see my latest edit, you need to put * in the function parameters to pass in a pointer, and use & when calling the function. * after a variable declaration means declare this variable as a pointer, * before a variable name means dereference this pointer, & means use pass this variables memory location – Eamonn McEvoy Apr 01 '11 at 17:36

score 0 · Answer 3 · answered Apr 01 '11 at 17:48

You mentionned an operator= overload, on a class which contains pointers. So let's go up one level, and see what it is: I suspect that what you really need is a copy constructor (first), and then use the swap idiom. Something like:

QtreeNode::QtreeNode( QtreeNode const& other )
    : north( other.north == NULL ? NULL : new QtreeNode( *other.north ) )
    , east( other.east == NULL ? NULL : new QtreeNode( *other.east) )
    , south( other.south == NULL ? NULL : new QtreeNode( *other.south) )
    , west( other.west == NULL ? NULL : new QtreeNode( *other.west) )
{
}

QtreeNode& QtreeNode::operator=( QtreeNode const& other )
{
    QtreeNode tmp( other );
    swap( tmp );
    return *this;
}

void QtreeNode::swap( QtreeNode& other )
{
    std::swap( north, other.north );
    std::swap( east, other.east );
    std::swap( south, other.south );
    std::swap( west, other.west );
}

What you definitely don't want to do is delete the existing nodes before you've successfully copied the new tree; that's a sure recepe for undefined behavior---usually in the form of double deletes.

And you don't need a special function for the copy; the copy constructor above is recursive, and will handle everything for you.

score 0 · Answer 4 · edited May 23 '17 at 11:47

Looks like the poster's problem is implementing copy assignment (operator=). I'd suggest that you implement your operator= in terms of copy constructor i.e. copy and swap. Take a look at this example in SO. Look at how operator= is implemented there.

This will pretty much give you operator= for free if you already implemented your copy constructor so you don't need to implement this "nodeCopier" just for the sake of copy assignment.

score 0 · Answer 5 · answered Apr 01 '11 at 19:03

When specifing "const" on a pointer it can apply to one or two things. The pointer can be constant (i.e., the address the pointer contains cannot change) or the value pointed to by that address can be constant, or both of them can be constant. See here for discussion.

const QtreeNode *       p1 = <value>;  // non-constant pointer to     constant value
      QtreeNode * const p2 = <value>;  //     constant pointer to non-constant value
const QtreeNode * const p3 = <value>;  //     constant pointer to     constant value

As shown in your question, the nodeCopier function takes a reference to a constant QtreeNode. From the error message we can see the compiler is looking for a nodeCopier that takes reference to a constant pointer to QtreeNode

Qtree::nodeCopier(Qtree::QtreeNode* const&)

In other words the type of *(source.root) doesn't match the type of the nodeCopier function's formal parameter. You can fix the problem by changing the definition of nodeCopier or by changing the actual parameter you pass to the nodeCopier call.

Functions with pointers

5 Answers5