nonremovable
nonremovable

Reputation: 836

Theoretical clarification regarding maps and iterators

If I have a class with a map as a private member such as

class MyClass
{
  public:
    MyClass();
    std::map<std::string, std::string> getPlatforms() const;
  private:
    std::map<std::string, std::string> platforms_;
};

MyClass::MyClass()
        :
{
  platforms_["key1"] = "value1";
  // ...
  platforms_["keyN"] = "valueN";
}

std::map<std::string, std::string> getPlatforms() const
{
  return platforms_;
}

And in my main function would there be a difference between these two pieces of code?

Code1:

MyClass myclass();
std::map<std::string, std::string>::iterator definition;
for (definition = myclass.getPlatforms().begin();
     definition != myclass.getPlatforms().end();
     ++definition){
  std::cout << (*definition).first << std::endl;
}

Code2:

MyClass myclass();
std::map<std::string, std::string> platforms = myclass.getPlatforms();
std::map<std::string, std::string>::iterator definition;
for (definition = platforms.begin();
     definition != platforms.end();
     ++definition){
  std::cout << (*definition).first << std::endl;
}

In Code2 I just created a new map variable to hold the map returned from the getPlatforms() function.

Anyway, in my real code (which I cannot post the real code from but it is directly corresponding to this concept) the first way (Code1) results in a runtime error with being unable to access memory at a location.

The second way works!

Can you enlighten me as to the theoretical underpinnings of what is going on between those two different pieces of code?

Upvotes: 3

Views: 101

Answers (3)

JSF
JSF

Reputation: 5321

getPlatforms() returns the map by value, rather than reference, which is generally a bad idea.

You have shown one example of why it is a bad idea:

getPlatforms().begin() is an iterator on a map that is gone before the iterator is used and getPlatforms().end() is an iterator on a different copy from the same original map.

Upvotes: 7

Joe
Joe

Reputation: 380

A problem that you have is that you should be using const_iterator not iterator. This is because the function getPlatforms is const qualified, whereas the function in the map iterator begin() is not; you must use the const qualified const_iterator begin() const instead to explicitly tell the compiler you will not modify any members of the class.

Note: this is only the case for code 1, which should, by the way return const&

Upvotes: -2

utnapistim
utnapistim

Reputation: 27365

Can you enlighten me as to the theoretical underpinnings of what is going on between those two different pieces of code?

When you return by value, you return a deep copy of the data.

When you call myclass.getPlatforms().begin(); and myclass.getPlatforms().end(); you are effectively constructing two copies of your data, then getting the begin iterator from one copy and the end iterator from the other. Then, you compare the two iterators for equality; This is undefined behavior.

results in a runtime error with being unable to access memory at a location.

This is because definition is initialized, then the temporary object used to create it is deleted, invalidating the data the iterator pointed to. Then, you attempt to use the data, through the iterator.

Upvotes: 1

Related Questions