Having trouble deleting vector of pointers

Question

I have a manager class holding a vector of pointers to a virtual base class to allow a variety of child classes to be stored there. In the destructor of this manager class, I want it to cycle through all the pointers it holds and delete them. However, I have tried a number of methods I've come across and the program keeps crashing during the execution.

The current code I have is shown below:-

for (std::vector<GameState*>::iterator it = gamestates_.begin(); it != gamestates_.end(); ++it){
    delete *it;
    it = gamestates_.erase(it);
}

One thing I haven't tried yet is using unique_ptr but I'm sure this should be able to handle it without using them. Please correct me if I'm wrong.

Edit: I'm aware I should clear the vector after the loop but this is what I've come to after trying every normal method of deleting the pointers. It doesn't seem to like the delete command.

Answer 1

Erasing an element from the vector invalidates the iterator, so you can't continue iterating afterwards. In this case, I wouldn't erase elements within the loop; I'd clear the vector afterwards:

for (auto it = gamestates_.begin(); it != gamestates_.end(); ++it){
    delete *it;
}
gamestates_.clear();

Although, if this is in the destructor and the vector is about to be destroyed, there's no point clearing it either.

If you do need to erase within a loop (perhaps because you only want to erase some elements), then you need a bit more care to keep the iterator valid:

for (auto it = gamestates_.begin(); it != gamestates_.end();){ // No ++ here
    if (should_erase(it)) {
        it = gamestates_.erase(it);
    } else {
        ++it;
    }
}

One thing I haven't tried yet is using unique_ptr but I'm sure this should be able to handle it without using them. Please correct me if I'm wrong.

If you do want to manage dynamic objects by steam by this, then make sure you follow the Rule of Three: you'll need to implement (or delete) the copy constructor and copy-assignment operator to prevent "shallow" copying leaving you with two vectors that try to delete the same objects. You'll also need to take care to delete the objects in any other place that removes or replaces them. Storing smart pointers (or the objects themselves, if you don't need pointers for polymorphism) will take care of all these things for you, so I would always recommend that.

I'm aware I should clear the vector after the loop but this is what I've come to after trying every normal method of deleting the pointers. It doesn't seem to like the delete command.

The most likely cause is that you're not following the Rule of Three, and are accidentally trying to delete the same objects twice after copying the vector. It's also possible that GameState is a base class and you forgot to give it a virtual destructor, or that the pointers have been corrupted by some other code.