unique_ptr and polymorphism

Question

I have some code that currently uses raw pointers, and I want to change to smart pointers. This helps cleanup the code in various ways. Anyway, I have factory methods that return objects and its the caller's responsibility to manager them. Ownership isn't shared and so I figure unique_ptr would be suitable. The objects I return generally all derive from a single base class, Object.

For example,

class Object { ... };
class Number : public Object { ... };
class String : public Object { ... };

std::unique_ptr<Number> State::NewNumber(double value)
    {
        return std::unique_ptr<Number>(new Number(this, value));
    }

std::unique_ptr<String> State::NewString(const char* value)
    {
        return std::unique_ptr<String>(new String(this, value));
    }

The objects returned quite often need to be passed to another function, which operates on objects of type Object (the base class). Without any smart pointers the code is like this.

void Push(const Object* object) { ... } // push simply pushes the value contained by object onto a stack, which makes a copy of the value
Number* number = NewNumber(5);
Push(number);

When converting this code to use unique_ptrs I've run into issues with polymorphism. Initially I decided to simply change the definition of Push to use unique_ptrs too, but this generates compile errors when trying to use derived types. I could allocate objects as the base type, like

std::unique_ptr<Object> number = NewNumber(5);

and pass those to Push - which of course works. However I often need to call methods on the derived type. In the end I decided to make Push operate on a pointer to the object stored by the unique_ptr.

void Push(const Object* object) { ... }
std::unique_ptr<Object> number = NewNumber(5);
Push(number.get());

Now, to the reason for posting. I'm wanting to know if this is the normal way to solve the problem I had? Is it better to have Push operate on the unique_ptr vs the object itself? If so how does one solve the polymorphism issues? I would assume that simply casting the ptrs wouldn't work. Is it common to need to get the underlying pointer from a smart pointer?

Thanks, sorry if the question isn't clear (just let me know).

edit: I think my Push function was a bit ambiguous. It makes a copy of the underlying value and doesn't actually modify, nor store, the input object.

Solution 1

Initially I decided to simply change the definition of Push to use unique_ptrs too, but this generates compile errors when trying to use derived types.

You likely did not correctly deal with uniqueness.

void push(std::unique_ptr<int>);
int main() {
    std::unique_ptr<int> i;
    push(i); // Illegal: tries to copy i.
}

If this compiled, it would trivially break the invariant of unique_ptr, that only one unique_ptr owns an object, because both i and the local argument in push would own that int, so it is illegal. unique_ptr is move only, it's not copyable. It has nothing to do with derived to base conversion, which unique_ptr handles completely correctly.

If push owns the object, then use std::move to move it there. If it doesn't, then use a raw pointer or reference, because that's what you use for a non-owning alias.

Solution 2

Well, if your functions operate on the (pointed to) object itself and don't need its address, neither take any ownership, and, as I guess, always need a valid object (fail when passed a nullptr), why do they take pointers at all?

Do it properly and make them take references:

void Push(const Object& object) { ... }

Then the calling code looks exactly the same for raw and smart pointers:

auto number = NewNumber(5);
Push(*number);

EDIT: But of course no matter if using references or pointers, don't make Push take a std::unique_ptr if it doesn't take ownership of the passed object (which would make it steal the ownership from the passed pointer). Or in general don't use owning pointers when the pointed to object is not to be owned, std::shared_ptr isn't anything different in this regard and is as worse a choice as a std::unique_ptr for Push's parameter if there is no ownership to be taken by Push.

Solution 3

If Push does not take owenrship, it should probably take reference instead of pointer. And most probably a const one. So you'll have

Push(*number);

Now that's obviously only valid if Push isn't going to keep the pointer anywhere past it's return. If it does I suspect you should try to rethink the ownership first.

Solution 4

Here's a polymorphism example using unique pointer:

vector<unique_ptr<ICreature>> creatures;

creatures.emplace_back(new Human);
creatures.emplace_back(new Fish);

unique_ptr<vector<string>> pLog(new vector<string>());

for each (auto& creature in creatures)
{
    auto state = creature->Move(*pLog);
}

View more solutions

Comments

[Sarang]

well new edit does help the cause. :)

[user1520427]

When I tried using the unique_ptr I tried passing by reference, like void push(const std::unique_ptr<Object>& object) but regardless, your answer clarifies things. For some reason I thought that getting the underlying pointer from a smart one was a dirty thing to do and should be avoided. But this is not the case?

[Jan Hudec]

@user1520427: Reference to pointer is the thing that will prevent polymorphism, because the uniqe_ptr specializations are unrelated types. You have to use reference to the object itself.

[Puppy]

@user1520427: No, it's fine to do that. But your solution would require copying the unique_ptr<Number> to a unique_ptr<Object> and then referring to that- which involves copying it.