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?

Answer 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.

Answer 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.

Answer 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.

Answer 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.

Answer 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.

Answer 6

There is absolutely no problem in using a vector of pointers. Most here are suggesting smart pointers, but I just have to say, there is no problem with using a vector of pointers without smart pointers. I do it all the time.

I agree with juanchopanza that the problem is your example is the pointers come from new foo(). In a normal completely-valid use case, you might have the objects in some other collection C, so that the objects will automatically get destroyed when C is destroyed. Then, in doing in the process of doing in-depth operations on the objects in C, you might create any number of other collections containing pointers to the objects in C. (If the other collections used object copies that would be time and memory wasteful, while collections of references is expressly forbidden.) In this use case, we never want to destroy any objects when a collection of pointers is destroyed.