1

I have a data structure represented by a vector of maps, all of which have the same template type. Inserting and reading works fine - however, for some reason, updates do nothing. I tried the methods described here, and they work fine - if I just use the map itself. However, when the map is in the vector, the element is found but not updated. Below, I have provided a minimal example.

#include <iostream>
#include <map>
#include <vector>
#include <optional>

std::vector<std::map<int, int>> vec = std::vector<std::map<int, int>>();

void insert_or_update( int key, int value ) {
    for ( std::map<int, int> map: vec ) {
        auto location = map.find( key );
        if ( location != map.end()) {
            location->second = value;
            std::cout << "This should update the value, but doesn't" << std::endl;
            return;
        }
    }

    // Insert, if no map currently contains the key
    std::cout << "This value is new" << std::endl;
    vec.back().insert( {key, value} );
}

int get_key( int key ) {
    for ( std::map<int, int> map: vec ) {
        auto location = map.find( key );
        if ( location != map.end()) {
            return location->second;
        }
    }

    std::cout << "This value doesn't exist yet" << std::endl;
    return 0;
}

int main()
{
   std::map<int, int> map = std::map<int, int>();
   vec.push_back( map ); 
   std::cout << get_key(3) << std::endl;
   insert_or_update(3, 3);
   std::cout << get_key(3) << std::endl;
   insert_or_update(3, 5);
   std::cout << get_key(3) << std::endl;
   std::cout << "Update in list failed, do it manually..." << std::endl;
   auto location = map.find( 3 );
   location->second = 5;
   std::cout << location->second << std::endl;

   return 0;
}

So my questions are:

  1. Why does this fail? I'm sure it's some kind of pointer logic I don't understand.
  2. What do I have to change to make it work?
Kjeld Schmidt
  • 771
  • 8
  • 19
  • 2
    In `for ( std::map map: vec ) { ` you are iterating over copies of maps. Replace that with `for ( std::map& map: vec ) { ` – Amadeusz May 14 '19 at 09:15

2 Answers2

3

Because this line:

for ( std::map<int, int> map: vec ) {

Is enumerating over every element in vec by value. It's making a copy of the map on each iteration of the for loop. Hence, you are inserting a new value into the copy and not the into item that's actually in the vector. This is likely what you want - enumerate the items by reference:

for ( std::map<int, int>& map: vec ) {

Or simply:

for ( auto& map: vec ) {

Do the same for the same line in get_key

selbie
  • 100,020
  • 15
  • 103
  • 173
3

Your function insert_or_update makes a copy of vec in the for loop:

for ( std::map<int, int> map: vec ) {
    auto location = map.find( key );
    if ( location != map.end()) {
        location->second = value;
        std::cout << "This should update the value, but doesn't" << std::endl;
        return;
    }
}

If you want to change the vec you need a reference instead of a copy:

for ( std::map<int, int> & map: vec ) {
    //...
}
doctorlove
  • 18,872
  • 2
  • 46
  • 62