Is this use of raw pointers in modern C++ bad practice?

Question

I want to hold a vector of Base class instances without object slicing (Such that I can also store instances of a child of Base without issue) while maintaining polymorphic behaviour without adding to the list by copying values, but rather by reference.

Consider the following source file:

#include <iostream>
#include <string>
#include <vector>
#include <memory>

class Entity
{
public:
    Entity(){this->edited = false;}
    virtual std::string name() = 0;
    bool edited;
};

class Player: public Entity
{
public:
    Player(): Entity(){}
    std::string name(){return "player";}
};

int main()
{
    std::vector<Entity*> entities;
    Player p;
    entities.push_back(&p);
    entities.at(0)->edited = true;
    Entity* ent = entities.at(0);
    std::cout << "ent = " << ent->name() << ", edited = " << ent->edited << ".\n";
    return 0;
}

I obtain the following output:

ent = player, edited = 1.

As the output shows (via printing out "player" and showing the change in 'edited'), polymorphic behaviour is maintained due to the raw pointer and I am able to edit members of the list without issue.

To clarify what I'm asking: Could I instead use an std::reference_wrapper to achieve the exact same behaviour? When I tried using a reference_wrapper, the same behaviour could not be achieved as pointers are required to achieve this polymorphic behaviour? If reference_wrappers are not a viable alternative, although I know full-well that the instance of Player that I added to the vector is a stack-variable, would it be sensible to instead use a shared_ptr? In my particular example I would favour a shared_ptr due to the fact that I want shared ownership of the members of the vector. Are there any better ways of achieving this behaviour?

Answer 1

Non-owning pointers is fine in modern c++. But if your pointer is owning, you better have a very good reason for using a raw pointer instead of a smart pointer. Even in the worst case, you should still be able to write your own smart pointer. Your example is a non-owning pointer.

Using std::reference_wrapper<T> doesn't gain you anything here. I've included your example, modified to use it. Notice that it's slightly more annoying to directly manipulate elements in your vector.

int main()
{
    std::vector<std::reference_wrapper<Entity>> entities;
    Player p;
    entities.push_back(p);
    entities.front().get().edited = true;
    Entity & ent = entities.front();
    std::cout << "ent = " << ent.name() << ", edited = " << ent.edited << ".\n";
    return 0;
}

A raw pointer is just a tool, like many others offered by c++. Don't be afraid to use it when it's the right tool. The most significant change brought on by modern c++ is that it has done away with owning raw pointers. The rule of thumb is if you have to remember to delete you are doing it wrong.

Note that std::vector<T>::front() is the preferred way of obtaining the first element and the use of std::vector::at is discouraged when you know your index is valid.