Why is using vector of pointers considered bad?

Question

Recently I've met with opinion that I shouldn't use vector of pointers. I wanted to know - why I cant?

For example if I have a class foo it is possible to do this:

vector <foo*> v;
v.push_back(new foo());

I've already seen some people down voting such practices, why is that?

Solution 1

Storing plain pointers in a container can lead to memory leaks and dangling pointers. Storing a pointer in a container does not define any kind of ownership of the pointer. Thus the container does not know the semantics of desctruction and copy operations. When the elements are being removed from the container the container is not aware how to properly destroy them, when a copy operation is performend no ownership semanctics are known. Of course, you can always handle these things by yourself, but then still a chance of human error is possible.

Using smart pointers leaves the ownership and destruction semantics up to them.

Another thing to mention is that containers are divided into non-intrusive and intrusive contaiers - they store the actual provided object instead of a copy so it actually comes down to a collection of pointers. Non intrusive pointers have some advantages, so you can't generalize that pointers in a container is something that should be avoided in all times, still in most cases it is recommended.

Solution 2

Using an vector of raw pointers is not necessary bad style, as long as you remember that the pointers do not have ownership semantics. When you start using new and delete, it usually means that you're doing something wrong.

In particular, the only cases where you should use new or delete in modern C++ code is when constructing unique_ptr's, or constructing shared_ptr's with custom deleters.

For example, assume that we have an class that implemented an bidirectional Graph, a Graph contains some amount of Vertexes.

class Vertex 
{
public: 
    Vertex();
    // raw pointer. No ownership
    std::vector<Vertex *> edges;
}

class Graph 
{
public:
    Graph() {};

    void addNode() 
    {
        vertexes.push_back(new Vertex); // in C++14: prefer std::make_unique<>
    }

// not shown: our Graph class implements a method to traverse over it's nodes
private:
    // unique_ptr. Explicit ownership
    std::vector<std::unique_ptr<Vertex>> vertexes;
}

void connect(Vertex *a, Vertex *b) 
{
    a->edges.push_back(b);  
    b->edges.push_back(a);
}

Notice how i have an vector of raw Vertex * in that Vertex class? I can do that because the lifetime of the Vertexes that it points to are managed by the class Graph. The ownership of my Vertex class is explicit from just looking at the code.

An different answer suggests using shared_ptr's. I personally dislike that approach because shared pointers, in general, make it very hard to reason about the lifetime of objects. In this particular example, shared pointers would not have worked at all because of the circular references between the Vertexes.

Solution 3

Because the vector's destructor won't call delete on the pointers, so it's easy to accidentally leak memory. A vector's destructor calls the destructors of all the elements in the vector, but raw pointers don't have destructors.

However, you can use a vector of smart pointers to ensure that destroying the vector will free the objects in it. vector<unique_ptr<foo>> can be used in C++11, and in C++98 with TR1 you can use vector<tr1::shared_ptr<foo>> (though shared_ptr has a slight overhead compared to a raw pointer or unique_ptr).

Boost also has a pointer container library, where the special delete-on-destruction behavior is built into the container itself so you don't need smart pointers.

Solution 4

One of the problems is exception-safety.

For example, suppose that somewhere an exception is thrown: in this case, the destructor of std::vector is called. But this destructor call does not delete the raw owning pointers stored in the vector. So, the resources managed by those pointers are leaked (these can be both memory resources, so you have a memory leak, but they could also be non-memory resources, e.g. sockets, OpenGL textures, etc.).

Instead, if you have a vector of smart pointers (e.g. std::vector<std::unique_ptr<Foo>>), then if the vector's destructor is called, each pointed item (safely owned by a smart pointer) in the vector is properly deleted, calling its destructor. So, the resources associated to each item ("smartly" pointed to in the vector) are properly released.

Note that vectors of observing raw pointers are fine (assuming that the lifetime of the observed items exceeeds that of the vector). The problem is with raw owning pointers.

Solution 5

I will talk specifically about a vector of pointers that's responsible for managing the lifetime of the pointed objects, because that's the only case where a vector of pointers is clearly a questionable choice.

There are much better alternatives. Specifically:

std::vector<std::shared_ptr<foo>> v;

and

std::vector<std::unique_ptr<foo>> v;

and

boost::ptr_vector<foo> v; // www.boost.org

The above versions tell the user how the lifetime of the objects is taken care of. Using raw pointers instead can possibly lead to the pointers being deleted either more or less than once, especially if the code is modified over time, or if exceptions become involved.

If you use an interface like ´shared_ptr´ or ´unique_ptr´, this self-documents the lifetime management for the user. When you use raw pointers you have to be clearly document how you handle the lifetime management of the objects, and hope that the right people read the documentation at the right time.

The benefits of using vectors of raw pointers are that you have more flexibility in how taking care of lifetime management, and that you can possibly get rid of some performance and space overhead.

View more solutions

Comments

[joppiesaus]

It won't call delete, but that's up to the programmer, isn't it? (I mean that you have to call delete when you delete the vector is necesarry)

[Admin]

@joppiesaus It's much harder to manage the lifetime of your pointers. i.e., copying the vector around.

[Wyzard]

Yes, you can avoid memory leaks by manually deleting all the elements, but that can be tricky to do (e.g. when exceptions are involved), and it's easy to forget or to get it wrong. It's not that containers of raw pointers are specifically discouraged, it's that raw pointers in general are discouraged. Use smart pointers unless you have a really good reason not to.

[Peter]

@nbro It's the usual answer because it's the right answer :)

[nbro]

It's not the right answer. Nobody knows the right things. Everything is relative.

[nbro]

But +1, because relativity sometimes is similar

[Fabian Keßler]

I completely disagree with all the answers here. The main reason, why this should be avoided, is that it adds extra indirection and reduces the cache locality.