C++ Correct way to free a vector of a custom class

Question

I have my custom class, like:

class MyClass {

   public:
        int i;
        std:string name;

        void DoSomeStuff();

}

and another class with a list of my custom class:

class MyClassList {

    public:
        std::vector<MyClasss> myClassList;
}

How shall be the list destructor in order to release all used vector space in memory:

MyClassList::~MyClassList 
{
    myClassList.clear();
    delete &myClassList;
}

Is that code right, redundant or wrong ?

Thanks for helping...

You don';t need delete in the destructor. When the destructor is called it will automatically clean up everything — itachi, Apr 28 '15 at 18:45
`delete &myClassList;` is outright incorrect. Only `delete` what you `new`. From the incomplete snippets you've shown us, `MyClassList` doesn't need a destructor definition. The implicitly defined one should be sufficient. — Praetorian, Apr 28 '15 at 18:45
Its wrong, dont do anything. vector manages its own memory, and your class member is not dynamically allocated. Did you `new`? No? Then don't delete. — Borgleader, Apr 28 '15 at 18:45
You don't even need to define a destructor, the compiler will make one for you, and it will work perfectly as you desire. — triple_r, Apr 28 '15 at 18:57
@Mendez Yes an empty `virtual ~MyClassList {}` if you intent to allow derivations. Otherwise, omit that (and make the class final (C++11)). — , Apr 28 '15 at 19:25

score 6 · Accepted Answer · answered Apr 28 '15 at 18:45

6

You don't need to do anything, just let it fall out of scope. RAII will ensure that the vector memory is cleaned up when your instance of MyClassList falls out of scope.

answered Apr 28 '15 at 18:45

Cory Kramer

114,268
16
167
218

Baum mit Augen · Answer 2 · 2015-04-28T18:54:37.343

This code is redundant (and as Cyber pointed out even incorrect because you are not allowed to delete the vector).

Note that you can and must delete only things allocated with new and that exactly once. Same applies to new[] and delete[]. Having a member that must be deleted would be a case where your own destructor would make sense, although leaving resource handling to resource handlers like smart pointers and std::vector is usually the way to go.

The implicitly generated destructor will destruct all members and bases in reverse order of construction, i.e. it will call all their destructors in the right order. So the destructor of std::vector will be called and release all resources it owns.

This principle applies to all well-designed classes and is known as RAII.

It is not redundant, it is plain incorrect and illegal. You cannot `delete` what you didn't `new` — Cory Kramer, Apr 28 '15 at 18:48

score 0 · Answer 3 · answered Apr 29 '15 at 01:30

Since everything is created on the stack, once the object you instantiate from your List class leaves scope its implicit destructor will call std::vector's destructor for you. If you want to make sure you can have your own destructor and use your member variables vector's.clear() to clear out its contents. The only time you need delete is when you are creating new memory on the heap! However I would not use new and delete unless if the new and delete calls are in your classes private methods and are both in the same method or if new is in the constructor and delete is in the destructor. Even then, it is still better to use shared_ptr<> and unique_ptr<> for if you do not use their release methods their destructors will do it for you automatically when the objects lose scope preventing memory leaks !

C++ Correct way to free a vector of a custom class

3 Answers3

Linked

Related