removing things from a pointer vector

Question

I have a vector of pointers like so:

vector<Item*> items;

I want to clear it. I've tried:

for (unsigned int i = 0; i < items.size(); i++)
    delete items.at(i);
items.clear();

,

while (!items.empty())
{
    delete items.back();
    items.pop_back();
}

,

while (!items.empty())
{
    delete items.at(0);
    items.erase(items.begin());
}

, and

while (!items.empty())
    delete items.at(0);

Every single one of these blows up for some reason or another, including deletion of already deleted objects and out of range vector iterators.

What do I do? I want to be able to reuse that same vector and add more Item pointers into it later. Just using delete without clearing still leaves junk pointers in there, right?

EDIT: Okay, I've switched to shared_ptrs. Now I have

vector<shared_ptr<Item> > items;

Yet, when I do items.clear(); , I get the error "vector iterators incompatible". What am I still doing wrong?

If you have a vector of pointers, use smart pointers (which you should be doing anyways). — chris, Apr 19 '12 at 22:46
There's `std::weak_ptr`, `std::unique_ptr` and `std::shared_ptr` since C++11. They leave the C++03 `std::auto_ptr` deprecated. — chris, Apr 19 '12 at 22:54
You need to be absolutely sure you really want to store pointers (or smart pointers) instead of objects themselves into your container (vector in this case). Don't do it if you don't need to. Read these questions: http://stackoverflow.com/questions/2693651/c-vector-of-objects-vs-vector-of-pointers-to-new-objects, http://stackoverflow.com/questions/141337/c-stl-should-i-store-entire-objects-or-pointers-to-objects — Bojan Komazec, Apr 19 '12 at 23:30
They all but the last seem OK. What is the problem you're encountering? — selalerer, Apr 19 '12 at 22:51

score 2 · Answer 1 · answered Apr 19 '12 at 23:13

I ran a test with all your ways of deleting stuff, and one of them simply doesn't work. See the code below for the comments on them.

To answer your question "what do I do," here is what I do when I seg-fault on a delete:
1) Make sure the memory is mine (do I know where the corresponding new is)?
2) Make sure I didn't delete the memory already (if I did, even if it WAS mine, it isn't now).

3) If you're pretty sure your seg-fault is caused by a single section of your code, break it out into a small test case in another project (kind of like you did in your question). Then play with it. If you had run your code examples up top in a small project you would have seen the seg-fault on the last one, and you would have noted the deletes worked in every other case. Breaking the code down like this would have let you know that you need to trace how you are storing these in your vector to see where you are losing ownership of them (via delete, or passing them to something that deletes them, etc...).

A side note: as others are saying, if you can use smart pointers do so, they will take care of the memory management for you. However, please continue your study here and understand how to use pointers that are dumb. There are times when you can not import boost, or have QT do your memory management for you. Also, there are times when you MUST store pointers in a container so don't be afraid to do that either (IE: QT developers STRONGLY suggest using pointers to store widgets instead of references or anything of the sort).

#include <vector>

using namespace std;
class Item
{
public:
    int a;
};

int main()
{
    vector<Item *> data;

    for(int x = 0; x < 100; x++)
    {
        data.push_back(new Item());
    }

    //worked for me, and makes sense
    for(int x = 0; x < 100; x++)
    {
        delete data.at(x);
    }
    data.clear();

    for(int x = 0; x < 100; x++)
    {
        data.push_back(new Item());
    }
    //worked for me, and makes sense
    while (!data.empty())
    {
        delete data.back();
        data.pop_back();
    }
    data.clear();

    for(int x = 0; x < 100; x++)
    {
        data.push_back(new Item());
    }

    //  //worked for me, and makes sense
    while (!data.empty())
    {
        delete data.at(0);
        data.erase(data.begin());
    }

    for(int x = 0; x < 100; x++)
    {
        data.push_back(new Item());
    }

//  //This one fails, you are always trying to delete the 0th position in
//  //data while never removing an element (you are trying to delete deleted memory)
//  while (!data.empty())
//  {
//      delete data.at(0);
//  }


    return 0;
}

score 0 · Answer 2 · answered Apr 19 '12 at 22:49

0

Either use a vector of smart pointers like this:

vector<shared_ptr<Item> > myVect;

Or use the Pointer Containers library in boost.

There may be a way to do this and re-use things, but it seems error-prone and a lot more work, especially considering Pointer Containers in boost is a header-only library.

answered Apr 19 '12 at 22:49

Kevin Anderson

5,815
3
29
52

Hmm, now how would I clear that vector? I have `vector > items;` now, and when I do `items.clear();` , the error I get is "vector iterators incompatible". – Pojo Apr 19 '12 at 23:24

score 0 · Answer 3 · answered Apr 19 '12 at 22:50

0

use boost::shared_ptr<Item> and they will be deleted when the vector is cleared, or the element is deleted.

answered Apr 19 '12 at 22:50

Keldon Alleyne

2,053
15
23

score 0 · Answer 4 · answered Apr 19 '12 at 22:50

What do I do?

Don't maintain a vector of pointers. Really, it's almost always a mistake and you are fighting against the design of the vector (RAII) which takes care of memory management for you. You now have to call delete on every pointer.

Do you really need a vector of pointers? If you really do (not just think you do, but it is actually a requirement for one reason or another), then use smart pointers.

The vector will dynamically allocate memory for you, just use it as it was intended to be used.

score 0 · Answer 5 · answered Apr 19 '12 at 23:15

It sounds as if you have the same pointer(s) repeated in your vector. To be sure you are only deleting them once just transfer them to a std::set and delete them there. For example,

std::set<Item*> s( items.begin(), items.end() );
items.clear();

while ( !s.empty() )
{
    delete *s.begin();
    s.erase( s.begin() );
}

score 0 · Accepted Answer · answered Apr 24 '12 at 20:27

Well, I did it. After a lot of time, a lot of aspirin, and a lot of lost hair, I finally figured out what the problem was. Turns out that I was calling a particular destructor earlier that contained the class which contained this vector of pointers. I had no idea that just calling a destructor would cause it to wipe all static data members. I hate c++ sometimes.

removing things from a pointer vector

6 Answers6