function return map value => iterator crashs prg, why?

hello,
i have an easy example that crashs.

but i did not know the reason.

the iterator != end() check doesnt work,
it prints me the 3rd element, but only 2 elements in the map.


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
#include <string>
#include <iostream>
#include <map>

class test_map
{
	public:
		test_map(void){internal_map.insert(std::pair<int, std::string>(1,"test"));internal_map.insert(std::pair<int, std::string>(2,"test"));}
		std::map<int, std::string>	get_map(void){return internal_map;};
	
	private:
		std::map<int, std::string>	internal_map;
};

int main(void)
{
	test_map a;
	std::cout << "a size: " << a.get_map().size() << std::endl;
	
	int i=0;
	for(std::map<int, std::string>::iterator it=a.get_map().begin(); it != a.get_map().end(); ++it)
		std::cout << "i: " << i << " first " << it->first << " second " << it->second << std::endl;
 }


i get the following output:
test_map.exe
a size: 2
i: 0 first 1 second test
i: 0 first 2 second test
i: 0 first 1852397404 second
... crash
can someone explain me that?

thanks
Last edited on
Your get_map function returns a copy of the internal map. So every time you're checking to see if it's time to finish ( it != a.get_map().end() ) you're using a whole new, completely different map object, so you're comparing iterators to different maps.

Don't return a copy of the map. Return a const ref to it, or some other such way of using the actual internal map, rather than a copy of it.

Obviously in a real class you wouldn't want to be just handing people direct access to internal variables, but for learning, this is all very educational.
hello,
in my opinion both function calls:
- a.get_map().begin() AND
- a.get_map().end()

creates two locale instances, and thats the reason why the iterator end doesnt work,
because first instance end != second instance end.

right?
Hi

Try this for loop, it is a safer way of iterating through a container, easier too


1
2
3
4
5
auto TheMap = a.get_map();
		
		for (auto&& item : TheMap) {
		    std::cout << "i: " << i << " first " << item.first << " second " << item.second << "\n";
		}


The auto&& is forwarding reference, you could have const auto& instead. item is one one of the items in the container.
Registered users can post here. Sign in or register to post.