Getting again a std::map changes the previous iterator

Viewed 71

I can't find any similar questions. The moment I call a getMap, the previously iterator seems to change:

//IF I COMMENT THE EVIL PRINT, THE PROBLEM DOES NOT OCCUR
std::cout << "EVIL PRINT" << std::endl;    
Something something;
auto mapElement = something.getTheMap().find("A");
std::cout << "Before: " << mapElement->first << std::endl;
something.getTheMap();
std::cout << "After: " << mapElement->first << std::endl << std::endl;

/****************************************************************************************/

//WITH SHARED POINTERS, THE EVIL PRINT IS NOT NECCESARY TO MAKE THE PROBLEM OCCUR
std::shared_ptr<Something> somePtr;
auto mapElement2 = something.getTheMap().find("A");
std::cout << "Before: " << mapElement2->first << std::endl;
something.getTheMap();
std::cout << "After: " << mapElement2->first << std::endl << std::endl;

OUTPUT:

EVIL PRINT
Before: A
After: B

Before: A
After: B

The complete code is runnable here https://coliru.stacked-crooked.com/a/66b48636a476ddb7

Is this a wanted behaviour? What is happening?

2 Answers

You did not include the most important parts in your question, namely

std::map <std::string, int> getTheMap() {
        return theMap;
}

getTheMap returns a copy, so getTheMap().find("A"); returns an iterator to a temporary object (that stops existing after the call finishes).
Therefore that iterator references an object that no longer exists, it is a dangling iterator. Dereferencing it (as you do with mapElement->first) invokes undefined behavior

The most idiomatic fix would be for getTheMap to return a reference, e.g.:

std::map <std::string, int>& getTheMap() {
        return theMap;
}

You have Undefined Behaviour because you refer to map outside of its lifetime.

getTheMap() returns map by value, which means you get a copy of the original map. You never save this copy anywhere, so the iterator becomes dangling immedetialy after it is created.

Something something;
auto mapElement = something.getTheMap().find("A"); //temporary map used here
// temporary map is gone and mapElement is invalid

Depending on what you want, you can return the map by reference(this will allow to modify internal map from the outside):

std::map <std::string, int>& getTheMap() {
    return theMap;
}

Or save the copy map to make sure it exists when iterator is used

auto map = something.getTheMap();
auto mapElement = map.find("A");
Related