Deleting a iterator inside a double loop :S

So yea... I know that if I erase a value inside an iterator, it becomes invalidated, and this is how you would normally fix it:
1
2
3
4
5
6
7
for(std::vector<int>::iterator it = list.begin(); it<list.end(); )
{
  if((*it) == 5)
    list.erase(it);
  else
    it++;
}

But what about a double loop? like this one:
1
2
3
4
5
6
7
8
9
//This would delete all duplicates
for(std::vector<int>::iterator it = list.begin(); it<list.end(); it++)
{
  for(std::vector<int>::iterator it2 = list.begin(); it2<list.end(); it2++)
  {
    if((*it) == (*it2))
      list.erase(it2);
  }
}
Your loop would actually delete everything.. because 'it2' is starting at the beginning, which means it will always erase the element when it gets to 'it'.

To answer your actual question: you'd do the same thing:

1
2
3
4
5
6
7
8
9
10
11
12
// to erase duplicates:
for(auto it = list.begin(); it != list.end(); ++it)  // <- use != instead of < for iterators
{
    auto it2 = it+1; // <- start at it+1, not at the beginning of the vector
    while(it2 != list.end())
    {
        if(*it == *it2)
            it2 = list.erase(it2);
        else
            ++it2;
    }
}
Also, why do you do it2 = list.erase(it2);?
For me, just doing list.erase(it2); works?
For me, just doing list.erase(it2); works?


It will only work with vectors, not with other containers.

When you erase an element, the iterator is invalidated. You cannot [safely] increment an invalid iterator. Therefore, erase will return a valid iterator that points to the element after the one you just erased.
So if i dolist.erase(it2); it wont work? (even if I use vector?)
It'll work with vectors, but you could argue it's bad practice (just like using < instead of != for the loop -- that won't work with other containers either)

My advice: do it the way I show. But if you really are against it for whatever reason you'll probably be okay as long as you are only using vectors.


Also... regardless of what you do... do not increment it2 after erasing the element. Otherwise you will skip the next element in the array.
> this is how you would normally fix it:

One would normally use the erase-remove idiom. http://en.wikipedia.org/wiki/Erase-remove_idiom

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
#include <iostream>
#include <vector>
#include <algorithm>
#include <unordered_set>

int main()
{
    std::vector<int> seq { 1, 4, 5, 7, 4, 5, 8, 5, 9, 0, 6, 5, 3, 2, 5, 8 } ;
    for( int v : seq ) std::cout << v << ' ' ; std::cout << '\n' ;

    // remove all values equal to 5
    seq.erase( std::remove( std::begin(seq), std::end(seq), 5 ), std::end(seq) ) ;
    for( int v : seq ) std::cout << v << ' ' ; std::cout << '\n' ;

    // remove all duplicates; can reorder sequence
    seq = { 1, 4, 5, 7, 4, 5, 8, 5, 9, 0, 6, 5, 3, 2, 5, 8 } ;
    std::sort( std::begin(seq), std::end(seq) ) ;
    seq.erase( std::unique( std::begin(seq), std::end(seq) ), std::end(seq) ) ;
    for( int v : seq ) std::cout << v << ' ' ; std::cout << '\n' ;

    // remove all duplicates; may not reorder sequence
    seq = { 1, 4, 5, 7, 4, 5, 8, 5, 9, 0, 6, 5, 3, 2, 5, 8 } ;
    std::unordered_set<int> set ;
    auto iter = std::remove_if( std::begin(seq), std::end(seq), // note: move is copy for int
                                [&set]( int v ) { return !set.insert(v).second ; } ) ;
    seq.erase( iter, std::end(seq) ) ;
    for( int v : seq ) std::cout << v << ' ' ; std::cout << '\n' ;
}

http://rextester.com/NBGXJ54507
Topic archived. No new replies allowed.