CODEWITHSUNDEEP
c++c++11dictionarymultidimensional-arrayauto
Bobog
Bobog

Reputation: 163

C++11 - Range-based for loop on a two-dimensional map

I have a two-dimensional map that I declared like that :

typedef std::map<std::string, std::map<std::string, Objective>> objectives_t;

I want to save the content of this 2d map into a file.

So I tried something like that, inspired by some code I found on the web :

for (auto const &subject : m_objectives) {
    for (auto const &objective : m_objectives[subject.first]) {
        //Print the objective
    }
}

But of course, this doesn't work. How should I do that ? I'm not really sure what are subject and objective (are they some iterators ?).

On the second line, I get :

error: passing 'const objectives_t {aka const std::map<std::basic_string<char>, std::map<std::basic_string<char>, Objective> >}' as 'this' argument of 'std::map<_Key, _Tp, _Compare, _Alloc>::mapped_type& std::map<_Key, _Tp, _Compare, _Alloc>::operator[](const key_type&) [with _Key = std::basic_string<char>; _Tp = std::map<std::basic_string<char>, Objective>; _Compare = std::less<std::basic_string<char> >; _Alloc = std::allocator<std::pair<const std::basic_string<char>, std::map<std::basic_string<char>, Obj|

Upvotes: 2

Views: 1687

Answers (2)

Yakk - Adam Nevraumont
Yakk - Adam Nevraumont

Reputation: 275370

m_objectives is const in the above code.

operator[] doesn't work on const maps, because it is defined to modify the map if the key is not found.

So that is what your error is. Note that if it somehow modified the map, the outer loop would be screwed up, so the map being const was somewhat fortunate. As you have noted, it wouldn't actually modify it in this case (as you are feeding it keys back from itself).

The [] would be inefficient, regardless, as it generates an extra map lookup, when you have the value portion right there.

The fix is simple:

for (auto const &subject : m_objectives) {
  for (auto const &objective : subject.second) {
    //Print the objective
  }
}

which is both correct (in that it doesn't use [] on a const map), and faster (in that if you un-const-ified the map m_objectives somehow, using [] while iterating over the map is both inefficient and error-prone).

Upvotes: 0

Cory Kramer
Cory Kramer

Reputation: 117856

Your outter loop is correct. But the inner should be iterating over subject.second

for (auto const &subject : m_objectives) {
    for (auto const &objective : subject.second) {
        // print out objective.second
    }
}

This is because after the first range-based for loop, subject has type

std::pair<const std::string, std::map<std::string, Objective>> const&

So each item is

subject.first    // std::string
subject.second   // std::map<std::string, Objective>

Then when you iterate over subject.second, your objective is now a 

std::pair<const std::string, Objective> const&

So again to pull apart the elements

objective.first    // std::string
objective.second   // Objective

Upvotes: 9

Related Questions