[Home] [Downloads] [Search] [Help/forum]

Register forum user name Search FAQ

Gammon Forum

Notice: Any messages purporting to come from this site telling you that your password has expired, or that you need to "verify" your details, making threats, or asking for money, are spam. We do not email users with any such messages. If you have lost your password you can obtain a new one by using the password reset link.
 Entire forum ➜ Programming ➜ STL ➜ list for loops
list for loops

It is now over 60 days since the last post. This thread is closed.     Refresh page

Pages: 1  2 

Posted by David Haley   USA  (3,881 posts)  Bio
Date Reply #15 on Sat 19 Feb 2005 02:03 AM (UTC)
Message
Ah yes, it's remove, not erase. Erase is for erasing given iterator(s) and remove is for values.
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
Top
Posted by Samson   USA  (683 posts)  Bio
Date Reply #16 on Sat 19 Feb 2005 03:32 PM (UTC)
Message 

   for( ich = ch->in_room->people.begin(); ich != ch->in_room->people.end(); )
   {
      rch = (*ich);
      ich++;


I've been noticing a pattern in crashes I'm weeding out when using std::list and figured I'd share what I've found so far.

It seems that iterating over a list like this is fine - so long as there's soemthing in it. Seems that every one of my crashes relating to lists dies in the for loop statement for them. So my guess is that while the list still has a valid iterator, the contents of the member are not. I'm not sure how else to go about making sure this doesn't happen since I was under the impression that if you did list.remove( member ) it took the member out of the list.

All of the ones I've found so far could potentially be empty lists, so I suppose I need to know: Do you have to verify the list is not empty before walking it ( with a list.empty() check ) ?
Top
Posted by David Haley   USA  (3,881 posts)  Bio
Date Reply #17 on Sat 19 Feb 2005 04:05 PM (UTC)
Message
No, when the list is empty, list.begin() == list.end() so the loop should not get past the condition part of the for(;;) statement.

Where are you calling remove?
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
Top
Posted by Samson   USA  (683 posts)  Bio
Date Reply #18 on Sat 19 Feb 2005 04:20 PM (UTC)
Message
In this particular case, it would be when the target dies in combat. Although they aren't technically removed until clean_char_queue takes them out. And I moved clean_char_queue down past all of the update handlers in update.c, so if checking the list to be sure it's empty first isn't going to be helpful, I've got a much nastier problem lurking. :(
Top
Posted by Nick Gammon   Australia  (23,046 posts)  Bio   Forum Administrator
Date Reply #19 on Sun 20 Feb 2005 12:23 AM (UTC)
Message
You can certainly iterate over empty lists, the design specifically allows for that, which is why begin () points to the the start of a list, and end () points to one past the end of a list, so that the condition begin () == end () is true for empty lists (or maps or anything else).


However you need to be cautious about removing things from a list you are iterating through. The safe way is to increment the iterator before doing the remove. There is an example in the forum here, here it is again:



  for (people_map::iterator j = people.begin (); j != people.end (); )
    {
    if (j->second.GetAge () == 100)
      people.erase (j++); // iterator is advanced before the erase occurs
    else
      ++j;                  // advance the iterator
    } // end of erase loop


Notice that the way it is written, things will work correctly here. :)
- Nick Gammon

www.gammon.com.au, www.mushclient.com
Top
Posted by Samson   USA  (683 posts)  Bio
Date Reply #20 on Sun 20 Feb 2005 01:13 AM (UTC)
Message
So what are the consequences of doing something like so then:


   for( ich = ch->in_room->people.begin(); ich != ch->in_room->people.end(); )
   {
      rch = (*ich);
      ich++;

      ch->in_room->people.remove( rch );

      ...rest of function...


It appears to me that as long as the iterator is advanced to a valid location before calling for removal that it should work. What's the advantage of using erase() ? And suppose when the erase() call is needed you don't know what the current position of the iterator is in the list?
Top
Posted by Greven   Canada  (835 posts)  Bio
Date Reply #21 on Sun 20 Feb 2005 01:45 AM (UTC)
Message
If you don't know the iterator for the list, you can use find to retreive it, so that no iteration is required at all.
Nobody ever expects the spanish inquisition!

darkwarriors.net:4848
http://darkwarriors.net
Top
Posted by Raz   (32 posts)  Bio
Date Reply #22 on Sun 20 Feb 2005 04:17 AM (UTC)
Message
Quote:
Most likely, yes, because VARIABLE is what he's using as the value of the iterator. So it makes sense to assign the value of the iterator to VARIABLE at every pass.


I said that because such tricks are usually error prone to the hidden logic of C++. It really doesn't make sense to assign it in the conditional, but I would guess that he would have no other way to assign the variable and get the form he wants. (I don't know if that made complete sense.)

Quote:
Using for_each is major a pain in the behind because every time you use it, you need to create an external function - and if you want to use local variables, you have to make a functor, and pass it all kinds of variables in the constructor - basically, you have an awful lot of overhead.


You're assuming inlining a function guarantees better performance, which is not true. It is better to leave it up to the compiler via a function object than it is to write code in explicitly by hand. The compiler is almost always a much better chooser of how to optimize the code than the programmer.

Quote:
IMHO for_each is not a good solution for specific applications; it's a very good solution if you want to provide a generic way of mapping over the elements of a container.


If you want a specific solution, a different std:: algorithm would be better than std::for_each, of course. std::for_each is just the most general way when there isn't a more specific solution available.
-Raz
C++ Wiki: http://danday.homelinux.org/dan/cppwiki/index.php
Top
Posted by Raz   (32 posts)  Bio
Date Reply #23 on Sun 20 Feb 2005 04:32 AM (UTC)
Message
Quote:
It appears to me that as long as the iterator is advanced to a valid location before calling for removal that it should work.


As long as the iterator is not referencing that element you should be ok. It is probably best to use erase() when you are using iterators and remove() when you are dealing just with values.

Quote:
What's the advantage of using erase() ?


erase() is probably a constant-time operation (I'm 99% sure, but I don't have my standard with me). remove(), OTOH, is linear and erases all the elements that compare equal to the value passed.

Quote:
And suppose when the erase() call is needed you don't know what the current position of the iterator is in the list?


Right.
-Raz
C++ Wiki: http://danday.homelinux.org/dan/cppwiki/index.php
Top
Posted by David Haley   USA  (3,881 posts)  Bio
Date Reply #24 on Sun 20 Feb 2005 05:48 AM (UTC)
Message
Quote:
You're assuming inlining a function guarantees better performance, which is not true. It is better to leave it up to the compiler via a function object than it is to write code in explicitly by hand. The compiler is almost always a much better chooser of how to optimize the code than the programmer.
You didn't understand my point. I'm not talking about optimization, I'm talking about a pain for the programmer to have to leave little functors lying around every time s/he wants to iterate over a list.
Quote:
It really doesn't make sense to assign it in the conditional, but I would guess that he would have no other way to assign the variable and get the form he wants.
I guess that it doesn't make sense if you think about the conditional as a test for continuing, but it certainly works. I wouldn't do it that way, but as far as I can tell it's the only way for him to use the macro the way he wants to.
Quote:
If you don't know the iterator for the list, you can use find to retreive it, so that no iteration is required at all.
Except for the iteration to find it. :P
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
Top
Posted by Greven   Canada  (835 posts)  Bio
Date Reply #25 on Sun 20 Feb 2005 06:06 AM (UTC)

Amended on Sun 20 Feb 2005 06:07 AM (UTC) by Greven

Message
Heh, the macro isn't exactly important, just handy for our purposes, as it does its job in fairly clean code. As for my use of iteration, I meant it as the verb, not the noun :)
Nobody ever expects the spanish inquisition!

darkwarriors.net:4848
http://darkwarriors.net
Top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.

79,735 views.

This is page 2, subject is 2 pages long:  [Previous page]  1  2 

It is now over 60 days since the last post. This thread is closed.     Refresh page

Go to topic:           Search the forum

[Go to top] top

Quick links: MUSHclient. MUSHclient help. Forum shortcuts. Posting templates. Lua modules. Lua documentation.

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

[Home]