norman
norman

Reputation: 5556

Is this the right way to write a destructor?

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? 
}

Upvotes: 0

Views: 384

Answers (5)

Digital Da
Digital Da

Reputation: 911

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.

Upvotes: 2

EdChum
EdChum

Reputation: 393863

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.

Upvotes: 0

Praetorian
Praetorian

Reputation: 109079

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.

Upvotes: 1

Ben Cottrell
Ben Cottrell

Reputation: 6110

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.

Upvotes: 0

Iceman
Iceman

Reputation: 4362

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.

Upvotes: 0

Related Questions