delete dynamically allocated object from std::vector

Question

Is this correct?:

std::vector<Enemy*> enemies;
enemies.push_back(new Enemy());

Enemy* enemy = enemies[0];
enemies.erase(enemies.begin() + 0);
delete enemy;

Answer 1

It works, yes, but it's not an ideal approach.

Firstly, adding 0 is just noise, you can remove that. But even better, just use pop_front(). Also, no need for the intermediate step, you can delete before removing.

But std::vector isn't good as popping from the front, especially if it's large (because the remaining elements need to be shifted to fill the void). If you don't need contiguous memory, use a std::deque instead. Or, if order doesn't matter, you can use something like this:

template <typename T, typename A>
void unordered_pop_front(std::vector<T, A>& vec)
{
    using std::swap;
    swap(vec.front(), vec.back()); // constant time

    vec.pop_back(); // constant time
}

It swaps the front element with the back element, then pops it off. Of course, order is not retained.

The other problem is with your approach to memory management. Anytime you have explicit clean up code, you've done something wrong. It should be done automatically.

Use either Boost's ptr_vector, or a std::vector of smart pointers. (Note: do not use std::auto_ptr in a container, it's broken in this regard.) For a quick smart pointer suggestion, use either std::unique_ptr (if your compiler supports C++0x), or std::/boost::shared_ptr.

Answer 2

std::vector<Enemy*> enemies;
enemies.push_back(new Enemy());

This isn't exception-safe. If push_back fails to allocate enough memory to accommodate the new pointer, then the Enemy object is leaked.

Using a vector of smart pointers can solve this, but failing that you should reserve the space in the vector before pushing back:

std::vector<Enemy*> enemies;
enemies.reserve(1);    // or more generally, enemies.reserve(enemies.size()+1);
enemies.push_back(new Enemy());

Now we know that push_back can't fail to allocate memory, and if reserve fails then the exception is thrown before the Enemy is created.

Answer 3

Certainly looks fine to me. You don't need the + 0 in the enemies.erase line, but aside from that, it's perfectly OK.

Answer 4

Yes, that's fine. You can simplify it a little:

delete enemies[0];
enemies.clear();

For removing the element, you could also use:

enemies.pop_back();

or (very similar to yours):

enemies.erase(enemies.begin());