vector destructor

Question

I have a class defined like:

Class A
{
public:
    int num;
    A *parent;
    vector<A *> children;
    ...

    // constructor without parameters
    A(void)
    {
        this->num = 3;
        this->parent = 0;
        for (int i=0;i<num;++i)
            children.push_back(new A(this,num-1));
    }

    // constructor with parameters
    A(A *a,int n)
    {
        this->num = n;
        this->children->parent = a;
        for (int i=0;i<num;++i)
            this->children.push_back(new A(this,this->num-1));
    }
};

now, the constructor works fine. there are some problem with destructor. currently, the destructor is defined as:

A::~A(void)
{
    if (this->parent!=0) this->parent = 0;
    for (int i=0;i<(int)children.size();++i)
        this->children[i]->~A();
    vector <A *> ().swap(this->children);
}

but every time when I debug it, it will break at:

void deallocate(pointer _Ptr, size_type)
    {    // deallocate object at _Ptr, ignore size
    ::operator delete(_Ptr);
    }

it looks like I cannot delete the pointer in the vector of this->children, is there any way that I can de-construct the class successfully?

Aside: why are you casting `children.size()` to `int`, when you could just declare the loop variable as `size_t`? — Fred Foo, Jun 01 '12 at 15:25
If you use a `vector >` you won't need any of that code *and* your program will be more robust. Is that an option? — Jon, Jun 01 '12 at 15:27
I'm *pretty sure* that this line won't compile: `this->children->parent = a;` — Robᵩ, Jun 01 '12 at 15:51
@larsmans: better to use `vector::size_type` instead of `size_t` directly. All STL containers have a `size_type` typedef, as they don't all use `size_t`. — Remy Lebeau, Jun 01 '12 at 19:12
@RemyLebeau: `size_type` is defined as `size_t` for the default allocator. While I applaud genericity and standards compliance, I do find `std::vector::size_type` too much typing. (`auto` is even better, though.) — Fred Foo, Jun 01 '12 at 22:13
@larsmans: depending on which STL implementation you use, `vector::size_type` might not be using the `size_type` of the default allocator, but may be using `size_t` directly, or something else entirely. — Remy Lebeau, Jun 01 '12 at 22:19
@RemyLebeau: I just looked it up in the C++ standard and it seems you're right. My mistake, sorry. — Fred Foo, Jun 01 '12 at 22:46

score 5 · Answer 1 · edited May 23 '17 at 11:49

5

Your destructor should be:

A::~A(void)
{
    for (size_t i=0; i < children.size(); ++i)
        delete children[i];
}

You should probably also look into copy constructors. Otherwise, code like this will fail:

{
    A foo;
    B bar = foo;
}

Becuase you will delete the same pointers twice.

One of these two posts may help you with understanding that: 1 and 2

edited May 23 '17 at 11:49

Community

1
1

answered Jun 01 '12 at 15:28

Bill Lynch

80,138
16
128
173

score 1 · Answer 2 · answered Jun 01 '12 at 15:27

in the destructor of A, you shouldn't call the destructors of child items explicitly, instead, just delete the elements of the vector. this should be sufficient for your purpose:

A::~A(void)
{
    for (size_t i=0; i<children.size(); ++i)delete children[i];
}

score 0 · Answer 3 · answered Jun 01 '12 at 19:10

Your constructor with parameters has a typo bug in it - this->children->parent will not compiler as children is not a pointer nor does vector have a parent member. I suspect you meant to use this->parent instead.

Your destructor is directly calling the destructor of each child object, which will destroy the child but NOT free its memory. You allocate the child objects with the new operator, so you need to use the delete operator to call the destructor and free the memory correctly.

On a side note, your num member is redundant. You can use children.size() wherever num is needed.

Try this instead:

class A 
{ 
private:
  static const int default_num = 3;

public: 
    A *parent; 
    vector<A *> children; 
    ... 

    // default constructor
    A() 
    { 
        parent = 0; 
        for (int i = 0; i < default_num; ++i) 
            children.push_back(new A(this, default_num-1)); 
    } 

    // constructor with parameters 
    A(A *a, int n) 
    { 
        parent = a; 
        for (int i = 0; i < n; ++i) 
            children.push_back(new A(this, n-1)); 
    } 

    ~A()  
    {  
        parent = 0;  

        for (vector<A *>::size_type i = 0; i < children.size(); ++i)  
            delete children[i];  
        /*
        alternatively:
        for (vector<A *>::iterator i = children.begin(); i != children.end(); ++i)  
            delete *i;  
        */

        children.clear(); // optional, will be handled when vector is implicitally destructed
    }  
};

vector destructor

3 Answers3