c++ map iterator doesn't start from the first item

Question

I'm trying to iterate over a map for some reason the iterator doesn't start from the first item.

const Noeud* origine = &noeuds.at(nomorigine);
map<string, Noeud>::iterator it;
origine->seeRoutes(); //Prints n5: n4 n6
it = origine->getRoutes().begin();
cout<<it->first<<endl; //Prints n4.
for(it = origine->getRoutes().begin(); it != origine->getRoutes().end(); it++){
    cout<<it->first<<endl; //Prints n6.
}

void Noeud::seeRoutes() const{
    cout<<getNom()<<": ";
    for(auto it = routes.begin(); it != routes.end(); it++){
        cout<<it->first<<" ";
    }
    cout<<endl;
}

I've tried using auto but the result is the same. What could be the cause of this problem?

Here is the class for Noeud:

class Noeud{
  public:
    string getNom() const;
    void setNom(const string& nom);
    void ajouterRoute(const string& nomRoute, const Noeud noeud);
    map<string, Noeud> getRoutes() const;
    void seeRoutes() const;

  private:
    string nom;
    map<string, Noeud> routes;
};

Similar mistake and answer as [here](https://stackoverflow.com/questions/30041907/can-i-use-nested-loops-with-vectors-in-cpp). — PaulMcKenzie, Dec 20 '17 at 03:03
@Davar I thought there was no difference between ++it and it++. Is it always better to pre-increment in a for loop? — Anthony Gauthier, Dec 20 '17 at 03:15
@AnthonyGauthier [Use `++i` if you don't have a specific reason to use `i++`](https://stackoverflow.com/a/24904/3309790) — songyuanyao, Dec 20 '17 at 03:29
@AnthonyGauthier The `++it` operator takes the `it`, increments `it` and then returns `it`. The `it++` takes the `it`, makes copy of `it`, increments `it` and then returns the copy. In the iterator for loop it doesn't matter, because you are looking at the `it`, not taking into account that it returned some copy, but it has still created a copy (I'm not sure if automatic optimization are able to catch this.). One example you can try: `int main() { int x = 0; int y = 0; std::cout << x++ << " " << ++y; }` http://en.cppreference.com/w/cpp/language/operator_incdec — ValentaTomas, Dec 20 '17 at 03:35
Perhaps don't do `map getRoutes() const;`, but instead create a `map::const_iterator getRoutesBegin() const;` and `map::const_iterator getRoutesEnd() const;`. — Eljay, Dec 20 '17 at 03:50

songyuanyao · Accepted Answer · 2017-12-20T03:18:22.313

3

getRoutes() returns by value, that means what origine->getRoutes() returns is a temporary which will be destroyed after full-expression. After that it becomes dangled, any dereference on it leads to UB.

You can use a named variable to avoid this problem. e.g.

map<string, Noeud> m = origine->getRoutes();
it = m.begin();
...

And note that for the same reason, in the for loop origine->getRoutes() is invoked twice and returns two irrelevant objects. The iterators got from them are incompatible, comparing them is ill-formed.

edited Dec 20 '17 at 03:18

answered Dec 20 '17 at 03:01

songyuanyao

169,198
16
310
405

It works, thank you. So the only way to avoid this problem is storing the map in a named variable before the loop? – Anthony Gauthier Dec 20 '17 at 03:16
@AnthonyGauthier Depending on the design, changing `getRoutes` to return-by-reference could solve the issue too. Make certain to return a valid reference. – songyuanyao Dec 20 '17 at 03:19

c++ map iterator doesn't start from the first item

1 Answers1