Deleting user-defined vectors - C++

Question

I have 2 classes, say A & B. Class B has a destructor of its own. Within class A, I have a vector of pointers to objects of class B. The vector is as follows:

vector<B*> vect;

In the destructor for class A, how do I retrieve memory? If I cycle through the vector, do I retrieve each object and use delete on every retrieved object? I tried that out in the destructor, but it segfaults.

Any help in solving this problem is most welcome. I am sorry but I cannot post the code.

Answer 1

Some other posts pointed out that you're better of using smart pointers instead of pointers. If you have to use pointer for any reason whatsoever you should delete them in a loop first.

for ( std::vector<B*>::iterator it = vect.begin(); it != vect.end(); ++it)
    delete (*it);
vect.clear();

edit: If your program segfault in the destructor then your code is wrong. Maybe you put stack element by adress in the vector, but to delete an object it has to be on the heap.

#include <iostream>
#include <vector>
#include <string>

class data {
public:
    std::string d;
    data(std::string _d) : d(_d) { }
};

class container {
public:
    std::vector<data*> c;
    container() { c.clear(); }
    void add (data *d) { c.push_back(d); }
    ~container() {
        for (std::vector<data*>::iterator it = c.begin(); it != c.end(); ++it)
            delete (*it); 
        c.clear();
    }
};

int main (int argc, char* argv[]) {

    typedef std::vector<std::string> sVec;
    typedef sVec::iterator sVecIter;

    std::vector<std::string> cmd (argv+1, argv+argc);

    {
    container k;            
    for (sVecIter it = cmd.begin(); it != cmd.end(); ++it)
        k.add(new data((*it)));

    }

    return 0;

}

This works without problem.

Answer 2

If A owns the things pointed to by vect, then it should be able to delete each item within vect. If it segfaults while doing so, then you have a bug somewhere in your code.

In general, though, you're better off using smart pointers. Boost's ptr_vector (part of Boost.Pointer Container is designed for your specific example, but a simple std::vector<std::tr1::shared_ptr<B> >will also work (albeit with more overhead and a more awkward syntax).

Answer 3

Yes if the items of type B* point to objects allocated on the heap, then for each item you should call delete on it.

Answer 4

Yes, you would loop the vector and delete each item. The problem is that you forgot the towel operator on line 42.

Answer 5

You want to avoid storing pointers in containers when you need to manage their lifetime.

If this is the one true owner and is guaranteed to be last to cleanup then I'd go with

std::vector<B> vect;

If you have different references and lifetimes that may overlap then shared_ptr would be better (std::tr1 or boost depending on compiler)

std::vector< boost::shared_ptr<B> > vect;

Answer 6

From your description it sounds like your vect member of class A should have lifetime ownership of the data pointed by the B*.

I would recommend just changing that declaration to

vector< std::tr1::shared_ptr<B> > vect;

EDIT: replaced auto_ptr with std::tr1::shared_ptr