How do you add abstract base class unique_ptrs to a map?

Question

I'm currently programming a game in C++. This game features a GameManager class. The GameManager class contains a map that holds pointers to game objects. I have defined a GameObject class that is an abstract class acting simply as an interface.

I have defined two classes that derive from the GameObject class: Enemy and Loot.

I want my GameManager class to contain a map of game objects, or rather, pointers to game objects. Because my GameManager owns these objects, I want the map to contain std::unique_ptr's.

However, I'm having a difficult time actually adding derived objects (e.g. Enemy and Loot) to this map.

I want my GameManager to iterate over the game objects and call the abstract methods. Essentially, my GameManager does not care whether or not something is an enemy, loot, or whatever, it just wants to be able to call the "draw" method as declared in the base class.

How would I go about adding a unique_ptr, that points to a derived class, to a map that contains unique_ptr's to the base class? My attempts so far lead to code I can't compile. I keep getting an error that states I am not allowed to dynamically cast a derived class pointer to a base class pointer.

I feel like this work fine if I was using raw pointers, but I'm intent on using smart pointers.

Code:

#include <memory>
#include <map>

class GameObject
{
public:
    virtual void draw() = 0;
};

class  Enemy : GameObject
{
public:
    void draw() {};
};

class  Loot : GameObject
{
public:
    void draw() {};
};

int main()
{
    std::map<int, std::unique_ptr<GameObject>> my_map;

    // How do I add an Enemy or Loot object unique_ptr to this map?
    my_map[0] = dynamic_cast<GameObject>(std::unique_ptr<Enemy>().get()); // doesn't compile, complains about being unable to cast to abstract class

    return 0;
}

Answer 1

dynamic_cast can only be used to convert between pointers, and references. GameObject is neither a pointer type, nor a reference type, so you cannot dynamic_cast to it.

You may have intended dynamic_cast<GameObject*> instead. however, you shouldn't dynamic_cast to a (pointer to) a base class. A pointer to derived type is implicitly convertible to the base class pointer. Use static_cast when implicit conversion is not desirable. Furthermore, that conversion is not possible either, since the cast is outside of any member function, and therefore cannot have access to the private base class.

Furthermore, you cannot assign a bare pointer to a unique pointer. To transfer ownership of a bare pointer to a unique pointer, you can use unique_ptr::reset. Howver, you should never store a pointer from unique_ptr::get into another unique pointer. Doing so will result in undefined behaviour when both unique pointers destructors attempt to destroy the same object. Luckily in this case the pointer is value initialized, and therefore null, so the mistake has technically no consequences. But did you use null pointer intentionally? I suspect not.

Inserting a unique pointer to derived object into a map of unique pointers to base is simple. Let ptr be a unique pointer to Enemy:

std::unique_ptr<Enemy> ptr = get_unique_pointer_from_somewhere();

Simply move assign the unique pointer:

my_map[0] = std::move(ptr);

Or, you could use emplace member function of the map.

Finally, destructor unique_ptr<GameObject> will have undefined behaviour if it points to a derived object. To fix, declare the destructor of GameObject virtual.

Answer 2

The first cause of an error message is that a class type can never be used as the type for a dynamic_cast. The target type of dynamic_cast must always be either a pointer to class type (meaning the result is null if the cast fails) or a reference to class type (meaning to throw an exception if the cast fails).

So improvement #1:

my_map[0] = dynamic_cast<GameObject*>(std::unique_ptr<Enemy>().get());

But this won't work because GameObject is a private base class of Enemy. You probably meant to use public inheritance, but (when using class instead of struct) you must say so:

class Enemy : public GameObject
// ...

Next we'll find that the = within the map statement is invalid. The left side has type std::unique_ptr<GameObject>, which does not have any operator= that can take a GameObject* pointer. But it does have a reset member for setting a raw pointer:

my_map[0].reset(dynamic_cast<GameObject*>(std::unique_ptr<Enemy>().get()));

Now the statement should compile - but it's still wrong.

Before getting to why it's wrong, we can make a simplification. dynamic_cast is needed for getting a pointer to derived class from a pointer to base class, or for many other type changes within a more complicated inheritance tree. But it's not needed at all to get a pointer to base class from a pointer to derived class: this is a valid implicit conversion, since every object with a derived class type must always contain a subobject of the base class type, and there's no "failure" case. So the dynamic_cast here can just be dropped.

my_map[0].reset(std::unique_ptr<Enemy>().get());

The next problem is that std::unique_ptr<Enemy>() creates a null unique_ptr, and no Enemy object is created at all. To create an actual Enemy, we can write instead either std::unique_ptr<Enemy>(new Enemy) or std::make_unique<Enemy>().

my_map[0].reset(std::make_unique<Enemy>().get());

Still wrong, and in a slightly tricky way. Now the problem is that the created Enemy object is owned by the temporary std::unique_ptr<Enemy> object returned by make_unique. The reset tells the std::unique_ptr<GameObject> within the map that it should own a pointer to the same object. But at the end of the statement, the temporary std::unique_ptr<Enemy> gets destroyed, and it destroys the Enemy object. So the map is left with a pointer to a dead object, which is invalid to use - not what you wanted.

But the solution here is that we don't need to mess around with get() and reset() at all. There is an operator= that allows assigning an rvalue std::unique_ptr<Enemy> to a std::unique_ptr<GameObject>, and it does the right thing here. It makes use of the implicit conversion from Enemy* to GameObject*.

my_map[0] = std::make_unique<Enemy>();

(Note if you had a named std::unique_ptr<Enemy>, you would need to std::move it to allow the assignment, as in my_map[0] = std::move(enemy_ptr);. But std::move is not needed above because the result of make_unique is already an rvalue.)

Now this statement is much shorter and more legible, and will actually do what you want.

A comment also suggested the possibility

my_map.emplace(0, std::make_unique<Enemy>());

This is also valid, but there's a possibly important difference: if the map already has an object with key zero, the = version will destroy and replace the old one, but the emplace version will leave the map alone and the just-created Enemy will be destroyed instead.