CODEWITHSUNDEEP
c++iterator
Steven Zhao
Steven Zhao

Reputation: 301

Iterator going overbound and causing seg fault

So I have an iterator iterating through a map with int as key and a custome object as value. While iterating through this map, the iterator always go over the amount of elements stored in the map. I fixed this by forcing the loop to stop when a counter value becomes bigger than the size of the map, but I am quite confused as to why I need to do that (I am pretty sure the solution just defeats the point of an iterator). Here's the section of the code causing problem. Patrons, Address and Library are all predefined class that are working (since the values print out normally). 

map<int, Patron>::iterator it;
        int counter = 0;
        for(it = library.getPatrons().getPatrons().begin(); it != library.getPatrons().getPatrons().end() && counter < library.getPatrons().getPatrons().size(); ++it){
            cout << library.getPatrons().getPatrons().size() << endl;
            cout << it->second.getName() << endl;
            cout << it->second.getAddress().getStreetAddress() << endl;
            cout << it->second.getAddress().getCity() << ", " <<  it->second.getAddress().getState() << " " << it->second.getAddress().getZip() << endl;
            counter++;
            cout << endl;
        }

if this loop is iterating through a one element map, without the counter < library.getPatrons().getPatrons().size() check, the following are outputed:

1 //the size of the map
sdfoji //the name of the owner of the address
sdfio //the street address
sdfio, oifds 77494 //the city, state, and zip code

1
sdfoji
sdfio
sdfio, oifds 77494

1
//seg fault here

as you can see, the iterator iterated through 2 non existent value, seemingly ignoring the condition it != library.getPatrons().getPatrons().end()

Upvotes: 0

Views: 94

Answers (1)

Pavel P
Pavel P

Reputation: 16843

Your loop does not clearly answer what's wrong. Most likely one of your getPatrons() methods return map by value and you end up comparing your iterator to end() that belongs to another instance of map and for that reason your iteration will always run out of bounds.

Here's easy fix to verify my guess:

map<int, Patron>::iterator it;
int counter = 0;
map<int, Patron> patrons = library.getPatrons().getPatrons();
for (it = patrons.begin(); it != patrons.end(); ++it) {
    cout << patrons.size() << endl;
    cout << it->second.getName() << endl;
    cout << it->second.getAddress().getStreetAddress() << endl;
    cout << it->second.getAddress().getCity() << ", " << it->second.getAddress().getState() << " " << it->second.getAddress().getZip() << endl;
    counter++;
    cout << endl;
}

If this fixes your problem you should change the function to return by reference (cosnt reference) or you may as well use range-based for-loop which would not have this problem even if you return map by value:

int counter = 0;
for (const auto& it : library.getPatrons().getPatrons()) {
    cout << it.second.getName() << endl;
    cout << it.second.getAddress().getStreetAddress() << endl;
    cout << it.second.getAddress().getCity() << ", " << it.second.getAddress().getState() << " " << it.second.getAddress().getZip() << endl;
    counter++;
    cout << endl;
}

Upvotes: 1

Related Questions