Is this the right way to write a destructor?

Question

Besides a lot of member functions, my Graph class has 3 main members: a vector of pointers to its vertices, called "vertex", a vector of pointers to its edges, called "edge", and an integer counter variable. I started/attempted to write the destructor to deallocate the memory from the vectors, but I'm not sure if I am doing it correctly. And what do I do about the counter? I tried to say "delete counter," but it's not a pointer (oops).

Graph<Object,Weight>::~Graph(){
        for(unsigned int i=0; i<vertex.size(); ++i){
                delete vertex[i]; }
        for(unsigned int j=0; j<edge.size(); ++j){
                delete edge[j]; }

        //counter? 
}

Could you have `vector` instead of `vector`, etc? Then they would clean-up themselves. — Peter Wood, Apr 23 '12 at 08:03

score 2 · Answer 1 · edited May 23 '17 at 12:12

2

Deleting depends on the way you allocated, see The difference between delete and delete [] in C++

As for your int member, since you didn't dynamically allocate it (you didn't use new) you don't have to delete it.

edited May 23 '17 at 12:12

Community

1
1

answered Apr 22 '12 at 16:01

Digital Da

891
1
10
23

Praetorian · Answer 2 · 2012-04-22T16:12:47.650

Assuming the type of vertex is std::vector<Vertex*> and you create the vector as follows:

vertex.push_back( new Vertex );

your clean up code looks correct.

But, I urge you not to do this. Declare vertex as std::vector<std::unique_ptr<Vertex>> and you don't need to worry about deleteing the individual vector members anymore.

If you're using Boost, you could also make vertex a boost::ptr_vector.

As for the counter variable, unless you're newing the counter somewhere during class construction, you don't need to delete it.

EdChum · Answer 3 · 2012-04-22T16:13:59.137

I'm assuming that vertex and edge are allocated using new? then what you have written is fine but if you declared your vertex and edge as an array and newed using new [] operator then you need to use the delete [] call instead of delete, if the counter was not declared as a pointer and newed then there is no need to delete the counter.

As a design decision you should consider declaring your vertex and edge objects as boost::shared_ptr or unique_ptr so that they are reference counted and when the Graph object goes out of scope they are automatically cleaned up so you don't even need to flesh out your destructor.

If you have c++11 then you can use the std versions and not need boost like std::shared_ptr and std::unique_ptr.

You dont need boost for that anymore, `std::unique_ptr` and `std::shared_ptr` are there in **C++11** :) — vivek, Apr 22 '12 at 16:11

score 0 · Answer 4 · answered Apr 22 '12 at 16:01

0

You can delete an array of pointers with:

del[] arr; // arr is array of pointers.

Whereas, if counter is from stack, the program will take care of it, and you as programmer don't need to worry about freeing that memory.

answered Apr 22 '12 at 16:01

Iceman

4,202
7
26
39

hmm... i think this code will crash my application if called: void foo() { void* array_of_pointers[12] = {}; delete[] array_of_pointers; } – Corwin Apr 22 '12 at 18:19

score 0 · Accepted Answer · answered Apr 22 '12 at 16:02

is counter a plain ordinary int variable? If so, you're not in charge of its lifetime.

The delete keyword is only to be used when you've created an object with the new keyword. (And even then, only when there isn't something else in your program, such as shared_ptr which is doing the delete for you)

When using new/delete you're taking over from the language and managing the lifetime/existance of an object yourself. ordinary variables are created and destroyed automatically, meaning you don't need to worry about them.

Is this the right way to write a destructor?

5 Answers5