0

I have a key, and a pointer to left and right nodes defined in a struct, in a class for a binary search tree.

I was getting parasoft errors within the copy helper function of the class, so was advised to change the code to:

BinaryTree::Node* BinaryTree::copyHelper(const Node* other)
{
    if(other == NULL)
    {
        return NULL; // If there's no Node to copy, return NULL.
    }
    else
    {
        //Node* newNode  = new Node; 

        typedef std::unique_ptr<Node> NodePtr;  
        NodePtr newNode(new Node); 

        if(newNode)
        {
            newNode->name  = other->name;
            newNode->left  = copyHelper(other->left); 
            newNode->right = copyHelper(other->right);
        }

        return newNode;
    }
}

Now I am getting an error on the return statement of the newNode:

IntelliSense: no suitable conversion function from NodePtr to BinaryTree::Node *

Any Ideas?

ildjarn
  • 62,044
  • 9
  • 127
  • 211
dev6546
  • 1,412
  • 7
  • 21
  • 40
  • Could you check your code sample please, at the moment your definition of `newNode` won't compile. – Matthew Walton May 02 '12 at 14:22
  • the ctor of unique_ptr is explicit – PlasmaHH May 02 '12 at 14:23
  • what do you want to return, `unique_ptr` or `Node*`? If you want Node*, then it does not need to have NodePtr in your function. – user2k5 May 02 '12 at 14:23
  • I used to use Node* newNode = new Node; but have been told to use the new bit of code http://stackoverflow.com/questions/10414869/parasoft-error-objects-to-manage-resources-should-be-used-instead-x-pointer I still want to return what I was returning before, but with using a smart pointer instead. – dev6546 May 02 '12 at 14:26
  • @LewisElliott: Of course the answerer also meant you to update the return type of the function. – Sebastian Mach May 02 '12 at 14:35
  • 2
    It seems you don't understand **why** you need a smart pointer and are trying to do this because *someone on the Internet* told you to. What they told you happens to be good advice but if you understand *why* that is good advice, you probably won't need to ask this question. – R. Martinho Fernandes May 02 '12 at 14:36

4 Answers4

3

Ahah. You can't convert a unique_ptr<T> to a T*. They're not the same type, and they don't have the same semantics particularly when they get copied. This is why unique_ptr<T> doesn't have a conversion operator to T*. T* isn't a class, so you can't rely on polymorphism to do the work for you like you can with returning a subclass of the declared return type (and in fact unique_ptr's semantic differences would mean that subclassing would be the wrong relationship anyway).

So you don't have a choice here - you have to return a unique_ptr<T> and have your callers deal with the consequences as they need to know it's a unique_ptr<T> and behaves like one. If you feel this is too much of a burden, I suggest reading up on smart pointers to understand better what they are and what they do for you.

You might also need to consider if it's the correct kind of smart pointer class, as maybe shared_ptr<T> has more suitable semantics for you.

Matthew Walton
  • 9,809
  • 3
  • 27
  • 36
  • *You can't convert a `unique_ptr` to a `T*`*. While you *shouldn't* you clearly can. There is no conversion, but `unique_ptr` has a `release` member function that will yield ownwership of the pointer. That being said, whether you want to do that or not is a completely different issue. – David Rodríguez - dribeas May 02 '12 at 14:47
  • That's why I didn't call it a conversion. – Matthew Walton May 03 '12 at 12:19
3

If I understand it copyHelper is an internal function, not part of the interface. If that is so, then the simplest solution is changing the signature to return a unique_ptr rather than a raw pointer.

The alternative is calling release on the unique_ptr inside copyHelper to give up ownership, and let the caller reclaim it in it's own unique_ptr (for that you will need to use reset), but there is no point in going through a raw pointer when you are going to store it in a unique_ptr in the very next step.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • Thank you, I've changed it, but as all of my other functions are expecting a raw pointer it has created many more errors. I think I'm going to leave my two parasoft erros in my solution, I only lose two marks for them out of a possible 100. – dev6546 May 02 '12 at 14:53
  • @LewisElliott if you find answers useful you can also upvote them. I see plenty of useful answers here... – juanchopanza May 02 '12 at 14:56
  • They are useful, but I don't have 15 reputation yet, Im new. so I cannot upvote. – dev6546 May 02 '12 at 14:58
2

This is your function:

BinaryTree::Node* BinaryTree::copyHelper(const Node* other) { ...}

It returns a BinaryTree::Node*, and it should be returning a unique_ptr<Node>:

std::unique_ptr<BinaryTree::Node> BinaryTree::copyHelper(const Node* other) { ...}
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
1

The return type of the function and the value you actually return have different types, and there exists no implicit conversion between what you return and what your function is declared to return.

Sebastian Mach
  • 38,570
  • 8
  • 95
  • 130