Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

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

0 views
Skip to first unread message

Angus

unread,
Jan 21, 2010, 1:51:49 PM1/21/10
to
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?

Victor Bazarov

unread,
Jan 21, 2010, 2:04:53 PM1/21/10
to
Angus wrote:
> 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
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Angus

unread,
Jan 21, 2010, 2:18:52 PM1/21/10
to

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

Thanks.

James Kanze

unread,
Jan 21, 2010, 3:42:37 PM1/21/10
to

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 Kanze

Victor Bazarov

unread,
Jan 21, 2010, 4:05:29 PM1/21/10
to

James,

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

Juha Nieminen

unread,
Jan 22, 2010, 4:57:55 AM1/22/10
to
Victor Bazarov wrote:
>>> 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.
>
> James,
>
> Could you elaborate why the second has undefined behaviour? Thanks!

I would like to know that too.

--- news://freenews.netfront.net/ - complaints: ne...@netfront.net ---

Yannick Tremblay

unread,
Jan 22, 2010, 10:31:33 AM1/22/10
to
In article <54632acd-653b-4602...@a15g2000yqm.googlegroups.com>,

Angus <angus...@gmail.com> wrote:
>m_contacts is a std::list<myobject>.

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.

Victor Bazarov

unread,
Jan 22, 2010, 10:35:51 AM1/22/10
to
Yannick Tremblay wrote:
> In article <54632acd-653b-4602...@a15g2000yqm.googlegroups.com>,
> Angus <angus...@gmail.com> wrote:
>> m_contacts is a std::list<myobject>.
>
> 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').

> [..]

James Kanze

unread,
Jan 23, 2010, 8:05:21 AM1/23/10
to
On Jan 21, 9:05 pm, Victor Bazarov <v.Abaza...@comAcast.net> wrote:
> James Kanze wrote:
> > On Jan 21, 6:51 pm, Angus <anguscom...@gmail.com> wrote:
> >> 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.

> 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.)

--
James Kanze

Yannick Tremblay

unread,
Jan 25, 2010, 10:00:28 AM1/25/10
to
In article <hjcgl1$dra$1...@news.datemas.de>,

Victor Bazarov <v.Aba...@comAcast.net> wrote:
>Yannick Tremblay wrote:
>> In article <54632acd-653b-4602...@a15g2000yqm.googlegroups.com>,
>> Angus <angus...@gmail.com> wrote:
>>> m_contacts is a std::list<myobject>.
>>
>> 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').

Agree.

But in that case, the OP could use a simple STL compatible smart pointer.

Unfortunately, in a lot of case, some_stl_container<object *> tend to
be a misguided attempt at saving copies or a misunderstanding of why
the STL uses copy semantic (or maybe I should say a refusal to
understand or accept)

Yannick


0 new messages