Skip to main content
GameDev.net gamedev.net
🔒 Locked

stl problem

Started by Brad8383 Oct 22, 2001 at 3:41 PM 7 replies 450+ views
Original Post
Brad8383
Brad8383
for some reason when i do: list::iterator p = PlayerList.Begin(); while(p != PlayerList.End()) { if(something) PlayerList.Erase(p); p++; } then it exits on erase. if i do erarse(p,p) then it wont erase it. Edited by - Brad8383 on October 22, 2001 4:43:34 PM
Oluseyi
Oluseyi
quote:

list::erase



iterator erase(iterator it);
iterator erase(iterator first, iterator last);

The first member function removes the element of the controlled sequence pointed to by it. The second member function removes the elements of the controlled sequence in the range [first, last). Both return an iterator that designates the first element remaining beyond any elements removed, or end() if no such element exists.

Erasing N elements causes N destructor calls. No reallocation occurs, so iterators and references become invalid only for the erased elements.

p becomes invalid (since it''s been erased) and points to an invalid location (just like end()). Try this instead:

list::iterator p = PlayerList.begin();
while(p != PlayerList.end())
{
  if(condition)
    p = PlayerList.erase(p);
}
onlinegmz
onlinegmz
The STL sucks and it also leaks memory...I suggest not using it at all. Take the time and write your own.
Oluseyi
Oluseyi
quote:
Original post by onlinegmz
The STL sucks and it also leaks memory...I suggest not using it at all. Take the time and write your own.


This statement is such a crock of shit that I''m convinced its author hasn''t extensively used the STL (or doesn''t know how to). The whole point of the STL is to avoid your having to write common structures and algorithms. I''m told the MSVC STL implementation is sub-par; I''ve never done the tests myself, but you can obtain alternative implementations that are much better such as STLPort.
Stoffel
Stoffel
Yup. But don't worry. Losers like that just fall behind the tech curve and wonder why they can't find a job.

PS: slight bug in your code, though it's mostly correct:
    
list<POINT>::iterator p = PlayerList.begin();
list<POINT>::iterator end = PlayerList.end (); // mo' efficient

while(p != end) // end is calculated only once

{
  if(condition)
    p = PlayerList.erase(p);
  else
    ++p; // without this, it would loop forever if condition

         // was false

}



Edited by - Stoffel on October 22, 2001 5:44:25 PM
Oluseyi
Oluseyi
I thought erase returned the value of the next valid element, or end() if there are no more valid elements.

In any case, thanks.
jwalker
jwalker
complete 
  
    
list<POINT>::iterator p = PlayerList.begin();
while(p != PlayerList.end()) // end is calculated only once

{
  if(condition)
    p = PlayerList.erase(p);
  else
    ++p; 
    
}


i think this would work

{ Stating the obvious never helped any situation !! }
Stoffel
Stoffel
look at the disassembly for that solution. end () is calculated each time the condition is evaluated. That was the reasoning behind my solution.

edit: Adding source code & disassembly for MSVC 6.0, in case anybody has questions.
  
#include <list>
#include <map>
using namespace std;

//typedef map<int, int> Container; // test w/ different container if you like

typedef list<int> Container;

void f1 (Container &c)
{
    // my way, added const just to be silly

    Container::iterator it = c.begin ();
    const Container::iterator end = c.end ();
    while (it != end)
    {
        if (rand () % 1)
            it = c.erase (it);
        else
            ++it;
    }
}

void f2 (Container &c)
{
    // other way

    Container::iterator it = c.begin ();
    while (it != c.end ())
    {
        if (rand () % 1)
            it = c.erase (it);
        else
            ++it;
    }
}

Disassembly: first call has five instructions in loop, second has six (extra cmp).

PUBLIC	?f1@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z	; f1
EXTRN	_rand:NEAR
;	COMDAT ?f1@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z
_TEXT	SEGMENT
_c$ = 8
?f1@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z PROC NEAR ; f1, COMDAT

; 11   :     Container::iterator it = c.begin ();

	mov	eax, DWORD PTR _c$[esp-4]
	push	esi
	push	edi
	mov	edi, DWORD PTR [eax+4]
	mov	esi, DWORD PTR [edi]
$L10311:

; 12   :     const Container::iterator end = c.end ();
; 13   :     while (it != end)

	cmp	esi, edi
	je	SHORT $L9008

; 14   :     {
; 15   :         if (rand () % 1)

	call	_rand

; 16   :             it = c.erase (it);
; 17   :         else
; 18   :             ++it;

	mov	esi, DWORD PTR [esi]
	jmp	SHORT $L10311
$L9008:
	pop	edi
	pop	esi

; 19   :     }
; 20   : }

	ret	0
?f1@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z ENDP	; f1
_TEXT	ENDS
PUBLIC	?f2@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z	; f2
;	COMDAT ?f2@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z
_TEXT	SEGMENT
_c$ = 8
?f2@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z PROC NEAR ; f2, COMDAT

; 23   : {

	push	esi
	push	edi

; 24   :     Container::iterator it = c.begin ();

	mov	edi, DWORD PTR _c$[esp+4]
	mov	eax, DWORD PTR [edi+4]
	mov	esi, DWORD PTR [eax]

; 25   :     while (it != c.end ())

	cmp	esi, eax
	je	SHORT $L9019
$L9018:

; 26   :     {
; 27   :         if (rand () % 1)

	call	_rand

; 28   :             it = c.erase (it);
; 29   :         else
; 30   :             ++it;

	mov	esi, DWORD PTR [esi]
	cmp	esi, DWORD PTR [edi+4]
	jne	SHORT $L9018
$L9019:
	pop	edi
	pop	esi

; 31   :     }
; 32   : }

	ret	0
?f2@@YAXAAV?$list@HV?$allocator@H@std@@@std@@@Z ENDP	; f2
_TEXT	ENDS



Edited by - Stoffel on October 23, 2001 7:03:17 PM

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.