# for each, in - copying std::map elements?

## Recommended Posts

Having a little time to mess around with coding I sat down and opened my input engine and found that it's no longer responding to keypresses. (I swear the thing was working when I archived it.)

I followed it in the debugger and manually simulated a keypress, but something is wrong with this blasted VS2010 'for each' loop:

void KeyMap::update() {
for each(auto key in m_keys) {
key.second.update();
}
}


There's only one element in the std::map 'm_keys'. I follow the debugger into the update() call and have it set the object state to indicate that the key is pressed. When I come back out of the function keys.second has the correct state, but if I examine m_keys, the element there does not have the correct state. So apparently this loop is copying the std::pair objects from the map here? I disabled the copy ctor for the object's class and sure enough I get a compiler error. If I comment out the loop body I still get the error, so it's the loop making the copy, not the std::pair.

The MSDN docs for the loop make the cryptic comment 'When identifier is a tracking reference, you can modify the element.'

Gosh, thanks, MS. How the hell do I get it to work by reference rather than trying to copy?

I suppose I can just do it the old fashioned way, but I do like the simplified syntax.

Any advice would be appreciated. Thank you for your time anyone that responds.

Edited by Khatharr

##### Share on other sites
Shouldn't that be "auto& key in m_keys" if you don't want copies?

##### Share on other sites

That would be 'pointer to' in that case, wouldn't it?

...

Ah no, I get it. Because it's on the left.

Compiler doesn't like it.

1>c:\users\khatharr\desktop\khatengine\khatengine\cl_keymap.cpp(13): error C2662: 'VirtualKey::update' : cannot convert 'this' pointer from 'const VirtualKey' to 'VirtualKey &'
1> Conversion loses qualifiers

Wait a minute, why the hell is the VK marked const?

Let me see if I can track that down.

Okay, now I'm really confused. m_keys is declared:

std::map<unsigned int, VirtualKey> m_keys;

But this thing is trying to const it for that loop. What's up with that?

...

Same problem with this:

void KeyMap::update() {
std::map<unsigned int, VirtualKey>::iterator iter = m_keys.begin();
iter->second.update();
return;
//for each(auto& key in m_keys) {
//key.second.update();
//}
}

Edited by Khatharr

##### Share on other sites

The description here of the for each method has been corrected at the bottom of the page:

for each returns read only objects There is an error under the section Remarks in paragraph 1.

It is not possible to update the elements in a collection by updating the element that is returned by the for each command. The for each command returns an object by value, not by reference.

This should not lead to a const error:

void KeyMap::update() {
std::map<unsigned int, VirtualKey>::iterator iter = m_keys.begin();
iter->second.update();
return;
}
}


##### Share on other sites

I think you mixed C++ and C# syntax.

The C++11 for range syntax is

for(element : container)

so try

for(auto& key : m_keys) {
key.second.update();
}

Fixed code blocks

Edited by Rattrap

##### Share on other sites
No, the problem is that MSVC (maybe not 2012 but not sure of that) if they support for each at all, only support the for each ( ... in ...) syntax. And apparently even that is only half the deal because the const-only requirement is not a property of proper for each loops.

##### Share on other sites

I can highly recommend http://www.boost.org/doc/libs/1_53_0/doc/html/foreach.html if you dont mind getting boost.

##### Share on other sites

VS 2012 definitely supports range based for-loops. I don't know if any VC before 2010 supported that weird proprietary "for each( ... in...)" syntax, so using it seems like a really bad idea if you ever plan to change/update your compiler. Apparently even if sticking with 2010, considering the annoying const-ness limitation.

The more or less "portable" alternative to using proper C++11 in VS2012 is the BOOST_FOREACH macro in boost or simply typing those few extra expressions for a regular for-loop.

##### Share on other sites
Couldnt you write a foreach function using lambdas?

##### Share on other sites
So I presume there's some good reason you can't move to VS2012 and get to use the standard C++11 range-for (along with saving a lot of other grief as well)?

In VS2010 it's possible to use the for_each algorithm, or write a small template of your own for slightly more readability, while sticking to the standard:
// standard library
for_each(v.begin(), v.end(), [&](type& x){
...
});
// own template
foreach(v, [&](type& x){
...
});
I think that's got most of the usability, if not the performance, of the VS nonstandard extension. If you want both performance and usability, BOOST_FOREACH.

##### Share on other sites

Try this

for(auto& iter : m_keys)
{
iter.second.update();
}


That would be 'pointer to' in that case, wouldn't it?

...

Ah no, I get it. Because it's on the left.

Yes and no...

int a;
int& a_reference = a;
int* a_pointer = &a;


The "&" will always be on the left.

##### Share on other sites

First: God dammit, Microsoft!

Thank you for pointing that out, Ashaman.

Second.

Yes and no...

int a;
int& a_reference = a;
int* a_pointer = &a;


The "&" will always be on the left.

I meant on the left side of the (implied) assignment. Thank you for the clarification, though. :)

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628349
• Total Posts
2982210

• 10
• 9
• 24
• 11
• 9