When to use "delete"?

Question

I want to store 10 Obj object in objList, but i don't know when is appropriate use delete in this case. If i use delete Obj; in the line where i note in below code, will the Obj still be stored in objList?

struct Obj {
    int u;
    int v;
};

vector<Obj> objList;

int main() {
    for(int i = 0; i < 10; i++) {
        Obj *obj = new Obj();
        obj->u = i;
        obj->v = i + 1;
        objList.push_back(*obj);
        // Should i use "delete Obj;" here? 
    }
}

Solution 1

Any object you create on the heap with new needs to be cleared up by delete. In your code, you are actually storing a copy in your collection.

objList.push_back(*Obj);

What this line is doing step by step is:

1. Indirecting a pointer to the underlying heap memory Obj occupies (returning the object Obj)
2. Calling the copy constructor to create a temporary copy
3. Storing the temporary copy in the collection

You do not need to create this initial Obj on the heap, a stack allocated local will suffice as @Luchian Grigore has pointed out.

Obj obj;
objList.push_back(obj);

You do not need to call delete on the copy in the collection, the STL collection will handle this memory itself when you remove the element, but you still will need to delete the original heap allocated object.

It would be better if you stored your objects by std::shared_ptr. That way delete would be called when all references to the Obj were removed.

std::vector< std::shared_ptr< Obj > > vec;
vec.push_back( std::make_shared( new Obj() ) );

Solution 2

Yes, you should.

There should be a corresponding delete for every new in your program.

But your program doesn't need dynamic allocation:

Obj obj;
obj.u = i;
obj.v = i + 1;
objList.push_back(obj);

Also, your syntax was wrong - objList.push_back(*Obj); // should be *obj, not *Obj

Solution 3

Think about what happens:

In your loop you create a new instance of Obj and assign some values to it. This instance is created on your heap - thus you have to free it afterwards. When you add the instance to the vector you implicitely create a copy of it - because you have a vector of objects, not of pointers. Thus the vector keeps its own copy of Obj. You are safe to delete your Obj instance.

BTW:

• you could even reuse the object instance and create and free it outside of your loop
• it's not necessary to allocate the Obj instance on the heap

Obj x; x.u =i; x.v = i+1; objList.push_back(x);

would also do

You should read some articles about smart pointers and smart pointer containers. Eg. boost::scoped_ptr, boost::shared_ptr, std::auto_ptr. Using these paradigms there's usually no need to call delete by yourself.

Solution 4

You wanna use the heap, so I suggest changing your vector declaration to vector of pointers, something like this.

vector<Obj *>objList;

Why? Because if it is a vector of Obj's, then the things you're storing are actually copies of the value Obj of the pointers you created. objList's items and the obj pointers you create are totally separated and unrelated! So when you,

delete obj

The things on objList are totally completely unaffected in any ways.
Moreover, vector<Obj> (no asterisk), stores its item as Obj and they are on the stack, they will disappear when your program goes out of scope!

Solution 5

Well, the simple rule is that each variable constructed by new should be clean up by delete.

However depending on your goal, you may not need write the delete yourself.

At home, when learning the gory details of memory management, you will probably write as many delete as you write new.

Professionals, however, never use delete in applicative code (*). delete is a code smell. It's the shortest way to memory leaks in the presence of exceptions. Professionals use RAII to deal with exceptions safely, and namely: smart pointers/smart containers.

(*) as opposed to library code, ie someone wrote that shared_ptr class one day.

View more solutions