0

editor is a vector::iterator object and list is obviously a vector.

I'm wondering why it will not (vector).erase() the first element of the list, and when I try to with only 1 item, it throws an exception.

for (editor = list.begin(); editor < list.end(); ++editor)
    if (*editor == title)           
        list.erase(editor);
fjardon
  • 7,921
  • 22
  • 31
D Maloid
  • 1
  • 3
  • 2
    `erase` is going to invalidate your iterator. [std::vector::erase](http://www.cplusplus.com/reference/vector/vector/erase/) – crashmstr Sep 16 '15 at 13:02
  • 4
    "and list is obviously a vector" – Kerrek SB Sep 16 '15 at 13:08
  • @KerrekSB At first, I didn't understand why you said that :) – fjardon Sep 16 '15 at 13:13
  • I was aware after I posted this question, someone would definitely scoff at my using of "list" as a name. ; ) Lesson learned. – D Maloid Sep 16 '15 at 13:18
  • @fjardon: Even though I find personal amusement in reading sentences like that, I think there's actually a serious meta lesson here, namely that you always have to respect that everybody sees the world from their own, unique perspective. Something that would seem ridiculous to one person might make perfect sense to some other person. – Kerrek SB Sep 16 '15 at 13:34

4 Answers4

4

Prefer algorithms over hand-written loops. None of you worries would be present if you simply do this:

list.erase(std::remove(list.begin(), list.end(), title), list.end());

P.S. on a side note, I strongly advice against naming your object as STD types (list is an std::type), and even more, naming them so that they match a different container type.

SergeyA
  • 61,605
  • 5
  • 78
  • 137
1

Erasing while iterating a vector is not as simple as it seems and can cause iterator invalidation.

The right way to do this without invalidating the iterator is:

editor = list.begin();
while (editor < list.end()) {
    if (*editor == title)
        list.erase(editor++);
    else
        ++editor;
}

This ensures that the iterator is not invalidated when you erase the element.

Note that operator++ advances the iterator, but returns the former iterator position (the one you want to delete) to erase.

You can also use std::remove and std::remove_if in some constellation.

See this question for more info: std::vector iterator invalidation

Community
  • 1
  • 1
ArnonZ
  • 3,822
  • 4
  • 32
  • 42
  • Undefined behavior. Incrementing invalidated iterator is not allowed. – SergeyA Sep 16 '15 at 13:22
  • @SergeyA The iterator is not invalidated. The increment occurs before the erase, and operator++ returns its former value. please see operator++ documentation. – ArnonZ Sep 16 '15 at 13:26
  • True. Need a better glassess - I missed the increment altogether. I will upvote back once the lockdown expires (if you care about reputation, just remind me in 15 mins :) – SergeyA Sep 16 '15 at 13:35
  • Oh, there is no lockdown period - it's just locked for good. You need to edit it so that I can revote... – SergeyA Sep 16 '15 at 13:49
  • @SergeyA Thx. I edited the answer. – ArnonZ Sep 16 '15 at 14:03
  • Done :) I know how we newbies value the reputation :D – SergeyA Sep 16 '15 at 14:05
0

The problem isn't the erase; the problem is that you expect to use the same iterator afterwards (for the comparison with end()). It works if you immediately break out of the for loop after erasing the element.

MSalters
  • 173,980
  • 10
  • 155
  • 350
0

Are you sure it's the first one and not subsequent ones?

After an erase mutation to a vector, all iterators are invalidated (including end) so you have to re-build every iterator OR break and exit the loop as soon as you erase something.

for (editor = list.begin(); editor < list.end(); ++editor)
{
    if ( *editor == title )
    {
        size_t pos = editor - list.begin() ; // here's where we are now
        list.erase ( editor ) ;
        editor = list.begin() + pos ; // rebuild the iterator
    }
}

Or

for (editor = list.begin(); editor < list.end(); ++editor)
{
    if ( *editor == title )
    {
        list.erase ( editor ) ;
        break ; // stop iterating
    }
}
iAdjunct
  • 2,739
  • 1
  • 18
  • 27