I''m writing a program that creates a bunch of linked nodes, each node contains a position value of x, y, z and 6 pointers to type node: north, south, west, east, up, down. so after creating each node, I make a pointer to the node and store it in the STL map class, so i can access the node easily by position. Now when i want to delete all the heap space, i use iterator from the map class: void MapGrid::DeleteMap( void ) { map::iterator blockIterator; blockIterator = mapBlockStorage.begin( ); while( blockIterator != mapBlockStorage.end( ) ) { delete & blockIterator; blockIterator++; } } MapBlockPtr is a pointer to the MapBlock node. This is a non-recursive function for creating the bunch of MapBlock nodes: void MapGrid::BuildGrid( int x, int y, int z ) { stringstream tempPositionString; DeleteMap( ); mapBlockStorage.clear( ); currentPosition = new MapBlock( x, y, z ); tempPositionString << x << "," << y << "," << z; mapBlockStorage[tempPositionString.str( )] = currentPosition; cout << currentPosition << " " << mapBlockStorage[tempPositionString.str( )] << endl; blockCount = 1; BuildGridRec( currentPosition ); } the DeleteMap( ) part is giving a run time error: Debug Assertion Failed! File: dbgheap.c Line: 1044 Expression: _CrtIsValidHeapPointer(pUserData) Any suggestions on how to fix it? Thanks!

If you''re using templates in code you post on this board, you pretty much have to use these tags:
[ source ]
[ /source ]
(without the spaces)
Otherwise, the board swallows your templates.

I don''t know how you defined your map since you''re using typedefs, but I assume that MapBlockPtr is a "MapBlockPtr *"? If so, then your problem is here:

while( blockIterator != mapBlockStorage.end( ) ) { delete & blockIterator; // wrong blockIterator++; }

That''s actually deleting the iterator, not what it points to. Your iterator acts like a pointer, in that operator -> returns the object it contains. To dereference an iterator, you need to do *, not &.

Furthermore, since this is a map, the type that the iterator returns when dereferenced is a map&ltKey, T>::value_type, which is typedef''d as pair&ltKey, T&gt. This is so you can iterate through a list and get either the value _and_ the key that represents it.

Since iterator acts like a pointer to a pair, and you want to delete the second element of the pair, this is what you need to do:

while( blockIterator != mapBlockStorage.end( ) ) { delete blockIterator->second; blockIterator++; }

Since it''s difficult to see into STL containers while debugging, I usually explicitly dereference things to make sure I understand what''s going on, and can look into the structures if I want to:

// Stoffel''s way..while (mapBlockStorage.end () != blockIterator) // reversed, why?{ MapBlockPtr &ptr = blockIterator->second; // I could breakpoint the above line if I wanted to, look // into the ptr values, whatever, then.. delete ptr; ++blockIterator; // not blockIterator++, why?}

The reference doesn''t incur any overhead because it compiles-out in release builds. Explicitly dereferencing it to a type means there''s an extra check at compile-time; if your blockIterator''s type wasn''t a MapBlockPtr, or you tried to assign the address of the iterator to it, you would get a compile-time "cannot convert" error, rather than your run-time error.

I also made two other changes for specific reasons, but I''ll leave that up to you to figure out. This code actually might run slightly faster than yours, depending how your compiler handles return values from inlined operators. I''m not going to explain now because it''s late and I should really be working on my final project that''s due in 5.5 hours. =(

quote:

---

Original post by SpikyQube

I''m writing a program that creates a bunch of linked nodes, each node contains a position value of x, y, z and 6 pointers to type node: north, south, west, east, up, down.
so after creating each node, I make a pointer to the node and store it in the STL map class, so i can access the node easily by position.

Now when i want to delete all the heap space, i use iterator from the map class:

---

void MapGrid::DeleteMap( void ){ map<string, MapBlockPtr>::iterator blockIterator; blockIterator = mapBlockStorage.begin( ); while( blockIterator != mapBlockStorage.end( ) ) { delete & blockIterator; blockIterator++; }}

MapBlockPtr is a pointer to the MapBlock node.

This is a non-recursive function for creating the bunch of MapBlock nodes:

void MapGrid::BuildGrid( int x, int y, int z ){ stringstream tempPositionString; DeleteMap( ); mapBlockStorage.clear( ); currentPosition = new MapBlock( x, y, z ); tempPositionString << x << "," << y << "," << z; mapBlockStorage[tempPositionString.str( )] = currentPosition; cout << currentPosition << " " << mapBlockStorage[tempPositionString.str( )] << endl; blockCount = 1; BuildGridRec( currentPosition );}

the DeleteMap( ) part is giving a run time error:

Debug Assertion Failed!
File: dbgheap.c
Line: 1044

Expression: _CrtIsValidHeapPointer(pUserData)


Any suggestions on how to fix it? Thanks!

I fixed my code according to your message and everything works fine, Thanks a lot!
I''m still trying to figure out the reason for some of your other changes. Aren''t they pretty much the same?

I can tell you about one of the changes he made (so Stoffel has more time for his project

++blockIterator; // instead of blockIterator++

Pre-incrementing an iterator is more efficient than post-incrementing it, because post-increment returns the value of the iterator _before_ it''s incremented. This means that the old value has to be stored and returned, so an extra copy of the iterator has to be made before increment. Pre-incrementing simply increments the iterator and returns the new value, so no extra copy has to be made.

So, if you''re incrementing iterators, for example in for loops, use the pre-increment operator.

HTH

---

Some useful C++ links:
Free multiplatform ANSI C++ Standard Library implementation
Visual C++ STL fixes
Visual C++ 6.0 noncompliance issues
C++ FAQ Lite

Turned my project in so I''m feeling better. =)

OK, so I was smoking crack with swapping the conditional. I thought end only returned a const iterator, and if you switched it so that the end () was the lvalue, you could save yourself from accidentally assigning instead of checking in the condition. But end has a non-const version too (why? no idea), so nevermind.

However, doing ++it rather than it++ is more efficient. it++ requires a copy of an iterator for STL classes, and is therefore slower than using ++it. Unless you''re actually using postincrement in an expression (i.e. saveIt = it++, you should always use ++it rather than it++ to avoud the possibility of creating a memory copy needlessly.

Stoffel,

End has a non-const version so you can assign it into a non-const iterator ...

say you had code like this

// say curr is as an iterator into my deque datacurr = data.begin();while(curr != data.end()) { if(*curr == special_value) curr = data.end(); else { // do cool stuff here } }

obviously this example is contrived, but you see it''s not that strange ... I would often have 2 generic iterators (non-const) to denote a range ... and when you want the last one to be the end ... you don''t want to cast the const away (which is usally a sign of a mistake - like gotos used to be).

My 2 cents.

ALSO ... everyone ... follow Stoffel ... PLEASE begin using ++i (standard increment ... NOT pre increment) ... etc ... even though compilers have learned to optimize builtin types ... they don''t optimize user defined types ... which means people who erroneously use i++ (post increment) everywhere think your code looks wrong and ugly and change it ... which makes it run worse ... I will never understand why schools (my college included) think that it''s ok to treat them the same .. they are NOT or they wouldn''t exist .. c tried to use as few ideas as possible and felt the need for both to exist for their particular uses.

these three are equal logically:

(i = i + 1)
(i += 1)
(++i)

if they were in EXPRESSIONS (not statements) they would behave the same ... but this one:

(i++)

does NOT!

ok .. enough already.