What exactly should go into a destructor.

[yasar11732]

Hi,

I have two classes, one is for a directory, and another for a file. Directory class, has this data member (alongside others):

private:
    set<file *> files;

and this member function:

void directory::addFile(string filename)
{
    temp = new file(filename,this); // second argument indicates parent for file.
    files.insert(temp);
}

Here is what I wrote as a destructor:

directory::~directory ()
{
    set<file *>::iterator pos;
    for (pos = files.begin(); pos != files.end(); ++pos)
    {
        delete (*pos); // delete file object
    }
    delete [] &files; // delete set of pointers.
}

My question is: is this a correct approach. And another question, should I also delete other data members, namely:

    string name;
    directory *parent;

They both are set inside constructor. I am wondering if they get deleted automatically or not.

[LB]

Line 8 is incorrect! You should NEVER delete something you didn't allocate with new.

The destructors of those other objects will be called automatically just as their constructors were called automatically. Also you don't call the destructors explicitly unless you used placement new.

[yasar11732]

Ohh, I see. Thanks.

[clanmjc]

If you allocate memory for directory * parent in your constructor then yes you need to free/delete in your deconstructor. As a side note, I tend to er on the side of safety and check my pointers before deleting them, this will save you headache in the future...

e.g.

if(*pos)
   delete (*pos);

And to clarify, standard containers will automatically call the deconstructor, but it will not deallocate memory that was allocated for that object so you do need to free that memory like you have.

As L B said, line 8 is incorrect. files is just a std::set which will cleanup itself.

Lets say your set was this instead:

//...

std::set<file> * files;

Your deconstructor would look something like this...

//...
//... No loop, deconstructors for each file object called automatically...
//...

if(files)
{
   delete (files);
   files = NULL;
}
//...

[LB]

clanmjc, delete does nothing if the pointer is 0.

[clanmjc]

duly noted, I guess it's out of habit before using a pointer and to indicate if there might be a bug.