-1

I want to do a list of lists using stl but I have a problem when I try to look for an element and remove it from the list, I'm unable to find out the error.

I made a little example with randoms number:

int main(int argc, char** argv) {

list<list <int> > l;
srand ( time(NULL) );

list<list <int> >::iterator j;
list<int>::iterator i;
for(int n1=0;n1<10;n1++){
    list<int> aux;
    int r=rand()%10 +1;
    for(int n=0;n<r;n++){
        int r2=rand()%100;
        aux.push_back(r2);
    }
    l.push_back(aux);
}
for(j=l.begin();j!=l.end();j++){
    list<int> l2=(*j);
    for(i=l2.begin();i!=l2.end();i++)
        cout << (*i) << " -> ";
    cout << endl;
}
int num;
cout << "element to delete: ";
cin >> num;

// ===== problem should be here =====

for(j=l.begin();j!=l.end();j++){
    for(i=(*j).begin();i!=(*j).end();i++){
        if((*i)==num){
            (*j).erase(i);
        }
    }
}

//===================================

cout << endl;
for(j=l.begin();j!=l.end();j++){
    list<int> l2=(*j);
    for(i=l2.begin();i!=l2.end();i++)
        cout << (*i) << " -> ";
    cout << endl;
}

}

I really appreciate any help you can provide.

Kaekh
  • 1

1 Answers1

1

You're probably right: that loop is likely the problem.

It's because you erase from *j at the same time as iterating over it; erasing that element will invalidate an iterator to it.

Here is how to erase from a list whilst iterating over it.

As an aside, your code is very difficult to read.

Community
  • 1
  • 1
Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • thanks so much, I found the problem. I guess i had to search a bit more. (*j).erase(i++); – Kaekh Oct 25 '14 at 18:51
  • @Kaekh I don't know about that solution. Won't it skip testing the node after the current one? I.e., if your inner list had two values in a row to be erased, your solution would only erase the first one. – ooga Oct 25 '14 at 19:02
  • @Kaekh: Do ensure that you implemented the linked solution properly: you need to get rid of your original `i++` in the for loop definition, and introduce an `else` clause. – Lightness Races in Orbit Oct 25 '14 at 19:11
  • @LightnessRacesinOrbit Try this: `std::list lst {1,2,7,7,3,4,7,7,5}; for (auto i = lst.begin(); i != lst.end(); i++) if (*i == 7) lst.erase(i++);` – ooga Oct 25 '14 at 19:12
  • The inner part needs to be more like this: `if (*i == num) i=lst.erase(i); else i++;` (with the increment removed from the for loop, of course.) – ooga Oct 25 '14 at 19:13
  • @ooga: You have an extra `i++` in that loop. You know it's wrong, so I don't understand why you're showing it to me. Neither my answer nor the answers I linked the OP to tell him to use an extra `i++`. – Lightness Races in Orbit Oct 25 '14 at 19:25
  • @LightnessRacesinOrbit I just copied the answer he originally posted here in the first comment (although he removed the looping part afterwards). Anyway, end of discussion! – ooga Oct 25 '14 at 19:27
  • ok so I have to decrease the iterator after erase, but anyway that was an example code, on my main problem the list elements will be unique so I'll not have that problem. Thanks for your help guys :) – Kaekh Oct 26 '14 at 14:46
  • @Kaekh: No, decreasing it afterwards is wrong. You have an invalid iterator. Did you read the comments above and the code I linked to? What do you mean by "unique"? Uniqueness of elements isn't relevant here, I shouldn't think... – Lightness Races in Orbit Oct 26 '14 at 15:32