• Advertisement
Sign in to follow this  

std::map iterator trouble

This topic is 4683 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have the following code:
std::map<std::string, GUIElement*> m_Elements;

...

GUIElement *GUI::FindChildAtCoord(int x, int y)
{
	std::map<std::string, GUIElement*>::iterator itr;
	for(itr = m_Elements.end(); itr != m_Elements.begin(); itr--)
	{
		GUIElement *w = itr->second->FindChildAtCoord(1, 1);
		if(w)
			return w;
	}

	if(x >= m_Rect.left && x <= m_Rect.right &&
		y >= m_Rect.bottom && y <= m_Rect.top)
		return NULL;

	return NULL;
}

I get a Windows error (GUI.exe has encountered a problem and needs to close.) on the call to FindChildCoord(). The odd thing is if I put the call into the render function it works, and if I put the render function call (itr->second->Render(m_Rect.left, m_Rect.bottom);) in FindChildAtCoord it still fails. It looks like this function has a problem calling the iterators methods :S I even get the error in the GUIElement::FindChildAtCoord() method, which is almost identical, with the same line of code. What's going on here?

Share this post


Link to post
Share on other sites
Advertisement
I don't believe that x.end() is a valid iterator. Perhaps maps are different, but I know with vectors and lists x.end() returns an iterator to one past the end of your vector/list, which means you can't dereference it.

*edit: I think, to do what you want to do, you need to use the reverse iteratrors.


for(map::reverse_iterator ritr = x.rend(); ritr != x.rbegin(); --ritr) {}


CM

Share this post


Link to post
Share on other sites
If I am not mistaken, you are supposed to loop from .begin() to != .end(), so if you do from .end() to .begin(), you may need to try something like this:


GUIElement *GUI::FindChildAtCoord(int x, int y)
{
std::map<std::string, GUIElement*>::iterator itr = m_Elements.end();
itr--;
for( ; itr != m_Elements.begin(); itr--)
{
GUIElement *w = itr->second->FindChildAtCoord(1, 1);
if(w)
return w;
}

if(x >= m_Rect.left && x <= m_Rect.right &&
y >= m_Rect.bottom && y <= m_Rect.top)
return NULL;

return NULL;
}





Then when you do hit the begin, you still want to use that so you might want to reconsider this design.

Share this post


Link to post
Share on other sites
Isn't that an infinite loop? The recursion never breaks, but keeps calling itself.

Here is an example that has the same problem:

int func(int y)
{
int x = 15;
x = func(y-15);

if(y == 0)
return 0;
}



the break point is there, but it will never get called, because it calls the recursion before.


Or, at least, I think...

Share this post


Link to post
Share on other sites
Quote:
Original post by Conner McCloud
I don't believe that x.end() is a valid iterator. Perhaps maps are different, but I know with vectors and lists x.end() returns an iterator to one past the end of your vector/list, which means you can't dereference it.

CM


I've tried it both ways, and it works for the render method.

Edit:
Quote:
Original post by Drew_Benton
If I am not mistaken, you are supposed to loop from .begin() to != .end(), so if you do from .end() to .begin(), you may need to try something like this:

*** Source Snippet Removed ***

Then when you do hit the begin, you still want to use that so you might want to reconsider this design.


That worked. I was only swapping it in the GUI method, not the GUIElement method as well.

Thanks everyone [smile]

Share this post


Link to post
Share on other sites
Quote:
Original post by visage
Isn't that an infinite loop? The recursion never breaks, but keeps calling itself.

Here is an example that has the same problem:
*** Source Snippet Removed ***

the break point is there, but it will never get called, because it calls the recursion before.


Or, at least, I think...

Only if m_Elements keeps having stuff in it. Presumably, at the bottom you'll find yourself with an empty m_Elements map, which means that for loop doesn't do anything.

CM

Share this post


Link to post
Share on other sites
The program just stops when it reaches itr--; Though I think it's only when there are no elements in the map.
Even in the debugger it just won't let me passed that line. Strange.

Edit: This works correctly:
GUIElement *GUIElement::FindChildAtCoord(int x, int y)
{
std::map&lt;std::string, GUIElement*&gt;::iterator itr = m_Elements.end();
//itr--;
for( ; itr != m_Elements.begin(); itr--)
{
GUIElement *w = itr-&gt;second-&gt;FindChildAtCoord(x, y);
if(w)
return w;
}

if(x &gt;= m_Rect.left && x &lt;= m_Rect.right &&
y &gt;= m_Rect.bottom && y &lt;= m_Rect.top)
return this;

return NULL;
}


This does not:
GUIElement *GUI::FindChildAtCoord(int x, int y)
{
std::map&lt;std::string, GUIElement*&gt;::iterator itr = m_Elements.end();
//itr--;
for( ; itr != m_Elements.begin(); itr--)
{
GUIElement *w = itr-&gt;second-&gt;FindChildAtCoord(x, y);
if(w)
return w;
}

if(x &gt;= m_Rect.left && x &lt;= m_Rect.right &&
y &gt;= m_Rect.bottom && y &lt;= m_Rect.top)
return NULL; // The GUI was clicked

return NULL;
}


What am I missing here?

Share this post


Link to post
Share on other sites
I'd do a rewrite:


std::map<std::string, GUIElement*>::iterator itr;
for( itr = m_Elements.begin(); itr != m_Elements.end(); itr++ )
{
GUIElement *w = itr->second->FindChildAtCoord(1, 1);
if(w)
return w;
}



See if that works better.

Share this post


Link to post
Share on other sites
I see where I'm going wrong now. I would rather go from back to front, but if I do then the first element in the map is missed out. How can I get it to include m_Elements.begin() and then exit the loop?

Share this post


Link to post
Share on other sites
[edit] See Conner McCloud's post [wink]. I didn't see it either till now.

This should do it. I took it from your above post that works. Note due to formatting from above, make sure you check over all the operators and such.


GUIElement *GUIElement::FindChildAtCoord(int x, int y)
{
std::map<std::string, GUIElement*>::iterator itr = m_Elements.end();
for( ; itr != m_Elements.begin(); itr--)
{
GUIElement *w = itr->second->FindChildAtCoord(x, y);
if(w)
return w;
}
// This will check the begin now
GUIElement *w = itr->second->FindChildAtCoord(x, y);
if(w)
return w;

if(x >= m_Rect.left && x <= m_Rect.right &&
y >= m_Rect.bottom && y <= m_Rect.top)
return this;

return NULL;
}





Share this post


Link to post
Share on other sites
Did you see my edit regarding reversible iterators? I'm almost certain that is the proper way to go through any container backwards, so if you have some reason not to go from the begining to end, then that's what you should do.

CM

Share this post


Link to post
Share on other sites
If you want to iterate backwards then you need to use rend (which stands for reverse end) and similarly rbegin. They return a Reversible Iterator. You use ++ to move backwards.


std::map<std::string, GUIElement*>::iterator itr;
std::map<std::string, GUIElement*>::iterator rbegin = m_Elements.rbegin();
for(itr = m_Elements.rend(); itr != rbegin, ++itr) {
//etc
}





Also, to communicate more clearly I would use some typedefs to simplify the code


typedef std::map<std::string, GUIElement*> ElementMap;
typedef ElementMap::iterator ElementMapItr;

ElementMap m_Elements;
//...
GUIElement *GUI::FindChildAtCoord(int x, int y)
{
ElementMapItr itr = m_Elements.rend();
ElementMapItr rbegin = m_Elements.rbegin();
while(itr != rbegin)
{
//etc
++itr;
}
//etc
}



Share this post


Link to post
Share on other sites
Quote:
Original post by petewood
You use ++ to move backwards.

Oh...that I did not know. Guess it makes sense.

Then, when using standard algorithms like find_if, could one provide rend() and rbegin() rather than begin() and end() in order to get the algorithm to iterate backwards as well?

CM

Share this post


Link to post
Share on other sites
Quote:
Original post by Conner McCloud
Quote:
Original post by petewood
You use ++ to move backwards.

Oh...that I did not know. Guess it makes sense.

Then, when using standard algorithms like find_if, could one provide rend() and rbegin() rather than begin() and end() in order to get the algorithm to iterate backwards as well?

CM

Yes, but you would pass rbegin() as start iterator and rend() as end iterator, just line begin() is the start and end() is the end. rbegin(), altough it returns a reversible iterator to the last element, is the beginning of the reversed container, and reverse iterators are used to iterate in revserse.

Share this post


Link to post
Share on other sites
Quote:
Original post by Brother Bob
Quote:
Original post by Conner McCloud
Quote:
Original post by petewood
You use ++ to move backwards.

Oh...that I did not know. Guess it makes sense.

Then, when using standard algorithms like find_if, could one provide rend() and rbegin() rather than begin() and end() in order to get the algorithm to iterate backwards as well?

CM

Yes, but you would pass rbegin() as start iterator and rend() as end iterator, just line begin() is the start and end() is the end. rbegin(), altough it returns a reversible iterator to the last element, is the beginning of the reversed container, and reverse iterators are used to iterate in revserse.

Oh...I had this thing all sorts of backwards. I know I've used this in the past. I wonder if I did it right before, and have since forgotten, or if I just got lucky. Thanks for clearing this stuff up.

CM

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement