Deconstructors and lists

Posted by Greven on Thu 28 Jul 2005 03:58 AM — 9 posts, 41,535 views.

Greven Canada Thu 28 Jul 2005 03:58 AM #0

I have a class that has a deconstructor. In the deconstructor function, I remove it from the list that it is in with this:


OLC_BOUNTY_DATA::~OLC_BOUNTY_DATA()
{
        std::list<OLC_BOUNTY_DATA * >::iterator iter;
        OLC_BOUNTY_DATA * bounty;

        for (iter = olc_bounties.begin(); iter != olc_bounties.end(); iter++ )
        {
            bounty = (*iter);

            if ( bounty == this)
            {
                olc_bounties.erase(iter);
            }
        }
}

Can someone tell me what I'm doing wrong? It keeps crashing when iter is dereferenced to be assigned into bounty.

Darrik Vequir Thu 28 Jul 2005 12:45 PM #1

You want to add a 'return' after olc_bounties.erase call. the iterator behaves in an undefined fashion if you erase something from a list but continue iterating through it.

DV

Greven Canada Thu 28 Jul 2005 04:02 PM #2

... How did I miss that? Thanks.

David Haley USA Mon 01 Aug 2005 07:03 PM #3

Another thing you can do is to iterate over a copy of the list, removing from the original. That's something you'd need to do when you need to remove multiple elements. Of course, there is a whole new set of pitfalls, e.g. the copied list obviously won't have the same iterator pointers as the old list...

Darrik Vequir Tue 02 Aug 2005 01:54 PM #4

Haven't hit a situation where I can't just it = list.begin() after an erase... or, I should say, haven't had a situation where the list is big enough to matter if it has to reset and re-search 4-5 times.

I usually try to use other containers if I get into those situations.

DV

David Haley USA Tue 02 Aug 2005 05:09 PM #5

Well, right, you can always just restart the whole iteration when you erase. But as you imply, that's really not very efficient when you have large lists, and even less efficient when comparing elements is a slow operation. What other containers would you have in mind?

Darrik Vequir Wed 03 Aug 2005 12:54 PM #6

I've mostly used maps for any large list I may need. I can find SOME key to look it up by that avoids having to iterate through the entire list to find something! :).

then again, I've just barely gotten the database working on my scratch base, have the most rudimentary of object structure, so its possible I may have to come up with a better system in the future.

DV

Kiasyn Kelle Sun 25 Sep 2005 10:41 AM #7

I think you can use something like:

OLC_BOUNTY_DATA::~OLC_BOUNTY_DATA()
{
std::list<OLC_BOUNTY_DATA * >::iterator iter;
OLC_BOUNTY_DATA * bounty;

for (iter = olc_bounties.begin(); iter != olc_bounties.end(); )
{
bounty = (*iter);

if ( bounty == this)
olc_bounties.erase(iter++);
else
++iter;
}
}

Samson USA Sun 25 Sep 2005 04:33 PM #8


OLC_BOUNTY_DATA::~OLC_BOUNTY_DATA()
{
        std::list<OLC_BOUNTY_DATA * >::iterator iter;
        OLC_BOUNTY_DATA * bounty;

        for (iter = olc_bounties.begin(); iter != olc_bounties.end(); )
        {
            bounty = (*iter);
            iter++;

            if ( bounty == this)
            {
                olc_bounties.remove(bounty);
            }
        }
}

You can also do it like so. All of my std::list calls use the remove() member instead of erase() and I set them all up in a similar manner to the above. They all work just fine this way for multiple member removal.