0

Please consider this C++ code:

#include <iostream>
#include <map>


// Global vars
std::map<int, char *>OurMap;
std::map<int, char *>::iterator it;


// prototypes
void populate();
void iterate();

int main(){
    puts("Hello, world!");
    populate();
    iterate();
    return 0;
}

void populate(){
    //char temp[512] = {0};
    for (int i = 0; i < 10; i++){
        char *temp = (char *) calloc(512, sizeof(*temp) );
        snprintf(temp, 256, "temp stuff %d", i);
        OurMap[i] = temp;
        std::cout << "Value: " << OurMap.find(i)->second << std::endl;
    }
}

void iterate(){
    for (auto i : OurMap){
        printf("i: %d\n", i);
        std::cout << "Value: " << OurMap.find(i)->second << std::endl; // <- blows up 
    }
}

C++ maps are bit new to me. Why is iterate() blowing up? I am able to iterate the key, so how do I get the value from elsewhere? I am confused as to why std::cout works in populate() but not in iterate().

mappy.cc:34:42: error: no matching function for call to ‘std::map<int, char*>::find(std::pair<const int, char*>&)’

I am trying to figure out the easiest way I can use my C++ map later in call.

Update:

#include <iostream>
#include <map>


// Global vars
std::map<int, char *>OurMap;
std::map<int, char *>::iterator it;


// prototypes
void populate();
void iterate();

int main(){
    puts("Hello, world!");
    populate();
    iterate();
    return 0;
}

void populate(){
    char temp[512] = {0};
    for (int i = 0; i < 10; i++){
        snprintf(temp, 256, "temp stuff %d", i);
        OurMap[i] = temp;
        std::cout << "Value: " << OurMap.find(i)->second << std::endl;
    }
}

void iterate(){
    for (auto i : OurMap){
        printf("i: %d\n", i);
        auto value = i.second;
        std::cout << "Value: " << value << std::endl;
    }
}
struggling_learner
  • 1,214
  • 15
  • 29
  • 1
    If you are writing C++ what's with the `char*` and `calloc`? Get rid of all that and just use `std::string` which will simplify half your code – Cory Kramer Dec 16 '20 at 20:04
  • 1
    In your current edit `OutMap[i] = temp` will store a pointer to a function-local array that falls out of scope, you now have a dangling pointer in your map. Which is yet another reason you should use `std::string` as I recommended. – Cory Kramer Dec 16 '20 at 20:42
  • 1
    Please note that your program is adding the *same* pointer to the `map` multiple times: https://godbolt.org/z/WbMdTT . BTW, do you know the difference between [shallow copy and deep copy](https://stackoverflow.com/questions/2657810/deep-copy-vs-shallow-copy)? – Bob__ Dec 16 '20 at 21:00
  • thanks, I used a char * because in my real use, I'd be storing structs as values. If C++ maps makes copies for me, my initial structs can be on the stack - once it is inside the map, if it stays there by copy I am done - if this is what happens behind the scenes. Then again, I don't know enough at this point. – struggling_learner Dec 16 '20 at 21:00
  • @Bob__, thank you! I overlooked this. – struggling_learner Dec 16 '20 at 21:04

1 Answers1

3

When you iterate over a map using a range-based for loop, you are not getting iterators back, but instead the (key, value) pairs themselves. So

for (auto i : OurMap){
    printf("i: %d\n", i);
    std::cout << "Value: " << OurMap.find(i)->second << std::endl; // <- blows up 
}

Should simply be

for (auto i : OurMap){
    printf("i: %d\n", i.first);
    std::cout << "Value: " << i.second << std::endl;
}
Cory Kramer
  • 114,268
  • 16
  • 167
  • 218