Comparing contents of two std::lists

Question

What i'm trying to do here is to compare two lists of structures like this one below. And if two persons share at least for example 3 intrests they should be paired together and put into list of pairs. I'm starting with the first girl in the list and compare it to the boys, if a pair is found it puts them in a a pairlist and remove them from their respective boylist/girllist.

 struct Person {
        char name[30];
        enum gendertype gender;
        TableOfIntrests intrests; //The tableofintrests consists of an array with 6 containters representing six diffrent intrests.
    };

Anyway, the problem I'm having is that the program works maybe ~50% of the times matching the persons and creating pairs. The other ~50% the I get an error message saying "List iterator not dereferancable". I have google the error message but I can't figure out what to do. Maybe I'm thinking completely wrong or it can be done in a much better way, I don't know but any feedback is appreciated.

void pair_together(Personlist *girllist, Personlist *boylist, Pairlist *pairlist, int            least_number_of_intrests)
{

int equal_intrests = 0; 
Pair pair;
Person p, p2;
int testcount3=0;
std::list<Person>::iterator i = girllist->begin();
std::list<Person>::iterator end = girllist->end();

std::list<Person>::iterator i2 = boylist->begin();
std::list<Person>::iterator end2 = boylist->end();
while ((i  != end))
{
    testcount3=0;
    if(i2==end2)
        break;

    equal_intrests = number_of_equal_intrests(i->intrests, i2->intrests);   //number_of_equal_intrests return the number of intrests that the two persons shares.   




    if(equal_intrests >= least_number_of_intrests)
    {           
        printf("%s + %s, ", i->name, i2->name);
        printf("%d\n", equal_intrests);
        equal_intrests =0;

        create_person(&p, i->name, i->gender);
        create_person(&p2, i2->name, i2->gender);
        create_pair(&pair, p, p2);
        pairlist->push_back(pair);
        i =girllist->erase(i);
        i2 =boylist->erase(i2);//--
        i2=boylist->begin();



        testcount3=1;

    }

     else if(testcount3!=1)
    {
        i2++;

    }

     if((i2==end2) && (equal_intrests < least_number_of_intrests))
    {
        i++;
        i2=boylist->begin();

    }

      if(number_of_intrests(i->intrests) <least_number_of_intrests)//number_of_intrests returns how many intrests a person have, so if the person have less intrests than least_number_of_intrests the program just skips to the next person.
    {           
        i++;            
    }




}

}

The standard-library provides you with functions that help you to match the contents of your containers. Look [here](http://www.cplusplus.com/reference/algorithm/). — bash.d, Aug 05 '13 at 14:28
http://stackoverflow.com/questions/596162/can-you-remove-elements-from-a-stdlist-while-iterating-through-it — doctorlove, Aug 05 '13 at 14:34

score 1 · Accepted Answer · answered Aug 05 '13 at 14:32

Towards the end you have this

if((i2==end2) && (equal_intrests < least_number_of_intrests))
{
    i++;
    i2=boylist->begin();

}

if(number_of_intrests(i->intrests) <least_number_of_intrests)//number_of_intrests ...
{           
    i++;            
}

In the second if, you don't check if i!=end and it could, so i->intrests could well cause problems. Try this

if((i!=end) && number_of_intrests(i->intrests) <least_number_of_intrests)//number_of_intrests ...
{           
    i++;            
}

If I'm not missreading the programs output, this did the trick. — Torsten, Aug 05 '13 at 14:53

SaganRitual · Answer 2 · 2013-08-05T14:50:19.807

0

You're erasing from your lists while iterating over them, which confuses the iterators. Instead, make copies of your lists. Iterate over the originals, but erase from the copies. When you're done, discard the originals and keep the copies.

EDIT: You don't have to make copies; you're right about the way you're resetting your 'i' iterator: it is safe. But when you erase from the list, you DO need to set a new value for the 'end' variable.

edited Aug 05 '13 at 14:50

answered Aug 05 '13 at 14:28

SaganRitual

3,143
2
24
40

So, since "list: only the iterators and references to the erased element is invalidated [23.2.2.3/3]" see http://stackoverflow.com/questions/6438086/iterator-invalidation-rules this isn't a problem, surely? – doctorlove Aug 05 '13 at 14:41
I used the if-statement that you posted doctorlove and the program seems to be working now. – Torsten Aug 05 '13 at 14:42
@doctorlove But after erase(), 'end' is also invalidated. So you're going off the end of your list. I suppose you could just reset 'end' when you erase a girl entry. – SaganRitual Aug 05 '13 at 14:44
@Torsen I was double checking what might be invalidated by the erase. I've undeleted it, but there might be neater ways of writing your code – doctorlove Aug 05 '13 at 14:44
@GreatBigBore are you sure, for lists? – doctorlove Aug 05 '13 at 14:46
@doctorlove At the beginning of your code, you set the 'end' variable to the current end of your list. When you erase, the value of the 'end' variable doesn't change. It's now pointing to something that was previously the end of the list, but it is no longer the end--it's just garbage now. You're right about only references to the erased element--you don't need to make copies as I originally suggested. But when you erase, you DO need to set a new value for the variable 'end'. – SaganRitual Aug 05 '13 at 14:48

Comparing contents of two std::lists

2 Answers2