Iterating Multiple Multimaps

Question

I'am having problems while trying to iterate some maps. Basically i have a Deposit class. Each deposit class has a multimap containing a destination Deposit and a distance. (This will be used to create a graph). When i try to iterate all the maps i'm getting a segmentation fault error.

Here's the code:

for (int j = 0; j < deposit.size(); j++) {
    for (typename multimap< Deposit<Product>*, int>::iterator it = deposit.at(j)->getConnections().begin(); it != deposit.at(j)->getConnections().end(); it++) {
        cout << "From the depo. " << deposit.at(j)->getKey() << " to " << it->first->getKey()     << " with the distance " << it->second << endl;
    }
}

EDIT:

Deposit Class:

template<class Product>
class Deposit {
 private:
  multimap <Deposit<Product>*, int> connections;
 public:
  void addConnection(Deposit<Product>* dep, int dist);
  multimap <Deposit<Product>*, int> getConnections() const;
 }; 
(...)
template<class Product>
void Deposit<Product> ::addConnection(Deposit<Product>* depKey, int dist) {
this->connections.insert(pair<Deposit<Product>*, int>(depKey, dist));
}

template<class Product>
multimap < Deposit<Product>*, int> Deposit<Product> ::getConnections() const {
return this->connections;
}

Storage Class - This is where I populate the multimaps.

(...)
    ligs = rand() % 10;
    do{
        ligIdx = rand() % deposit.size();
        dist = rand() % 100;
        deposit.at(i)->addConnection(deposit.at(ligIdx), dist);
        ligs--;
    }while(ligs>0);
(...)

My deposit class has 2 subclasses. I dont know why the error occurs. Is there any problem with the iterator?

Thank you very much!!!

Answer 1

The problem you have is pretty nasty: getConnections() returns a multimap by value.

This means that successive calls to deposit.at(j)->getConnections() refer to different temporary copies of the original multimap. Thus the the iterator created on the begin of the first temporary copy, will never match the end of the second copy, without first accessing illegally some invalid places.

Two alternatives:

• if you want to iterate on a copy, make one local copy auto cnx = deposit.at(j)->getConnections(); and change your inner loop to iterate on cnx.

• if you intended to iterate on the original multimap, change the signature of getConnections() to return a reference.

By the way, if you use c++11 or higher, you could consider defining the iterator in a more readable way: for (auto it = ....) or even better, using the range-for syntax as proposed by Norah Attkins in her answer.