Best way to delete contents of a list list of (pointers)

A

Angus

m_contacts is a std::list<myobject>.

This way:
for (std::list<myobject*>::iterator it = m_contacts.begin();
it != m_contacts.end(); ++it){
delete *it;
m_contacts.erase(it);
}

Or this way:
std::list<myobject*>::iterator it = m_contacts.begin ();
while (it != m_contacts.end ())
{
delete *it;
it = m_contacts.erase (it);
}

Are there any advantages of one over the other?
 
V

Victor Bazarov

Angus said:
m_contacts is a std::list<myobject>.

This way:
for (std::list<myobject*>::iterator it = m_contacts.begin();
it != m_contacts.end(); ++it){

You can't increment the iterator once you have erased it. Drop the
increment from here.
delete *it;
m_contacts.erase(it);

Should be

it = m_contacts.erase(it);

or

m_contacts.erase(it++);
}

Or this way:
std::list<myobject*>::iterator it = m_contacts.begin ();
while (it != m_contacts.end ())
{
delete *it;
it = m_contacts.erase (it);
}

Are there any advantages of one over the other?

Considering that the former is actually fixed, then no, both are pretty
much the same. The fastest would actually be not to 'erase' in the loop
but use 'clear':

for (... // or while(...)
...)
{
delete *it;
++it; // or do it as part of 'for'
}
m_contacts.clear();

And although 'clear' is defined as calling 'erase(begin(), end())',
calling it *once* can still be better/faster than calling 'erase'
multiple times.

V
 
A

Angus

You can't increment the iterator once you have erased it.  Drop the
increment from here.


Should be

              it = m_contacts.erase(it);

or

              m_contacts.erase(it++);




Considering that the former is actually fixed, then no, both are pretty
much the same.  The fastest would actually be not to 'erase' in the loop
but use 'clear':

     for (... // or while(...)
        ...)
     {
         delete *it;
         ++it; // or do it as part of 'for'
     }
     m_contacts.clear();

And although 'clear' is defined as calling 'erase(begin(), end())',
calling it *once* can still be better/faster than calling 'erase'
multiple times.

V

Ah yes, a fatal bug in first one - missed that.

Thanks.
 
J

James Kanze

m_contacts is a std::list<myobject>.
This way:
for (std::list<myobject*>::iterator it = m_contacts.begin();
it != m_contacts.end(); ++it){
delete *it;
m_contacts.erase(it);
}
Or this way:
std::list<myobject*>::iterator it = m_contacts.begin ();
while (it != m_contacts.end ())
{
delete *it;
it = m_contacts.erase (it);
}
Are there any advantages of one over the other?

Well, technically, both are undefined behavior, so there's no
real way to choose. But in practice, I can't imagine the second
failing in anyway, where as the first will fail with a good
library implementation.
 
V

Victor Bazarov

James said:
Well, technically, both are undefined behavior, so there's no
real way to choose. But in practice, I can't imagine the second
failing in anyway, where as the first will fail with a good
library implementation.

James,

Could you elaborate why the second has undefined behaviour? Thanks!

V
 
V

Victor Bazarov

Yannick said:
I'll assume the missing * is a mistake here?

However, this is quite relevant and maybe the correct answer is that
it shouldn't be a mistake.

m_contacts is a private list of myobjects that belong to a class.
Either the individual myobjects belong to the class, in this case the
class can destroy them, or they don't and the class can't destroy
them.

If the collection fully belong to the class, then you may as well use
a std::list<myobject> instead of a std::list<myobject*> and your
problem is gone.

Not if the actual objects created using 'new' are polymorphic (of types
that are descendants of 'myobject').

V
 
J

James Kanze

Could you elaborate why the second has undefined behaviour?

Well, the obvious answer is: because the standard says so, but
then, you'd only want to know where:).

The standard makes is very clear that any object in a container
must be assignable and copiable. And that once you delete a
pointer, it is neither: any lvalue to rvalue conversion of that
pointer is undefined behavior. (And in an assignment or an
initialization of a non-class type, there is an lvalue to rvalue
conversion of the right hand argument.) So if you call delete
on a pointer before removing it from the container, you have
undefined behavior.

Formally. Reasonably, no implementation is going to copy
anything without a reason to do so, so from a practical point of
view, you're certainly safe. Not to mention that systems that
in fact can't copy a deleted pointer are very, very rare, if
they exist at all. In practice, it's one of those formalities
that just aren't worth worrying about. (I probably should have
put a smiley after my first sentence above. Even if it is
formally true.)
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top