Deleting a iterator inside a double loop :S

Aug 25, 2014 at 2:26am
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);
  }
}
Aug 25, 2014 at 3:03am
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;
    }
}
Aug 25, 2014 at 3:35am
Also, why do you do it2 = list.erase(it2);?
For me, just doing list.erase(it2); works?
Aug 25, 2014 at 3:40am
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.
Aug 25, 2014 at 3:55am
So if i dolist.erase(it2); it wont work? (even if I use vector?)
Aug 25, 2014 at 4:02am
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.
Aug 25, 2014 at 2:16pm
> 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.