Can memory be leaked when using vector of pointer

Question

I tried this:

....
vector<players*> player;
for (int i = 0; i<10; i++)
{
    player.push_back(new players());
}
...

And I wonder if I need to free memory for the vector? If so, how?

Answer 1

If you need to store pointers to things in a container, you should either store some sort of smart pointer (like std::tr1::shared_ptr or boost::shared_ptr) or use a container designed for storing pointers, like those found in the Boost Pointer Container Library.

If you do store bare pointers, you need to remember to delete the pointers in the container before the container is destroyed. This includes any time that an exception is thrown that might cause the container to be destroyed. Getting this right is tedious, bug-prone, and wholly unnecessary given the facilities mentioned in the previous paragraph.

Answer 2

since you are creating new players(), you will have to delete them. probably best to iterate through the vector deleting the players and then clean up your vector.

Answer 3

Yes, you do need to delete them yourself. The vector is only going to "destruct" the pointers (which does nothing).

If you can, use the Boost pointer containers library, and you won't have to worry about it. However, if you can't you need to wrap the container up. Consider an exception is thrown between the time the container is populated and the time its elements are deleted. You will not execute the element delete code and leak.

A simple wrapper might look like:

struct default_deleter
{
    template <typename T>
    void operator()(T* pPtr)
    {
        delete pPtr;
    }

};

template <typename T, typename Deleter = default_deleter>
struct container_wrapper
{
    typedef T container_type;
    typedef Deleter deleter_type;

    container_wrapper(container_type pContainer = container_type()) :
    container(pContainer)
    {}

    ~container_wrapper(void)
    {
         std::for_each(container.begin(), container.end(), deleter_type());
    }

    container_type container;
};

Use it like:

typedef std::vector<int*> vec_intptr;
typedef container_wrapper<vec_intptr> vec;

vec v;
v.container.push_back(new int); // and never worry about it again

This is a simple wrapper. Any pop_back(), erase(), etc. operations will have the wrong effect. I strongly suggest using Boost.

One may think of using a container of auto_ptr. Contrarily, this is a bad idea; the copy-semantics of an auto_ptr prevent it from working. The best option is to get rid of the dynamic allocation, if possible.