Calling erase() on an iterator, but not deleting correct element

Question

I am learning C++ and I am working my way double linked lists, but I noticed something very peculiar when I was trying to delete elements from my list.

Problem: I am inserting an element before the value of 2 in my list , numbers, and then I am trying to delete any element that has the value 1.

Expected: After I call erase() in my conditional statement in my first loop my list, numbers, should get adjusted. The only values that numbers should should contain 0,1234,2,3.

Observed: My list numbers contains the values 0,1,1234,2,3. It is as if nothing was erased.

Code Example:

#include "stdafx.h"
#include <iostream>
#include <list>

using namespace std;

int main()
{
   list<int> numbers; 

   numbers.push_back(1);
   numbers.push_back(2);
   numbers.push_back(3);
   numbers.push_front(0);

   list<int>::iterator it = numbers.begin();
   it++;
   numbers.insert(it, 100);
   cout << "Element: " << *it << endl;

   list<int>::iterator eraseIt = numbers.begin();
   eraseIt++;
   eraseIt = numbers.erase(eraseIt);
   cout << "Element: " << *eraseIt << endl;

   for (list<int>::iterator it = numbers.begin(); it != numbers.end(); it++)
   {
      if (*it == 2)
      {
         numbers.insert(it, 1234);
      }

      if (*it == 1)
      {
         it = numbers.erase(it);
      }
      else
      {
         it++;
      }
   }

   for (list<int>::iterator it = numbers.begin(); it != numbers.end(); it++)
   {
      cout << *it << endl;
   }

    return 0;
}

What can I try next?

Your loop is invalid. If you insert or erase in the list, it invalidates the iterator. — Barmar, Feb 08 '18 at 02:04
@Barmar No, [std::list::insert](http://en.cppreference.com/w/cpp/container/list/insert) doesn't invalidate iterators, and for `erase` OP has reassigned `it` with the returned iterator from `erase`. — songyuanyao, Feb 08 '18 at 02:13

songyuanyao · Accepted Answer · 2018-02-08T02:24:40.187

5

You should remove the it++ at the end of the for loop declaration, because it might be increased inside the for loop too; when it gets inscreased twice some elements will be skipped. i.e.

for (list<int>::iterator it = numbers.begin(); it != numbers.end(); )

LIVE

edited Feb 08 '18 at 02:24

answered Feb 08 '18 at 02:10

songyuanyao

169,198
16
310
405

Are you sure that after erase(it) is executed (it) will be a valid iterator? – AndreyS Scherbakov Feb 08 '18 at 03:00
1

@AndreySScherbakov Yes after `erase` the iterator becomes invalid; that's why OP's reassigning it with the returned iterator of `erase`. – songyuanyao Feb 08 '18 at 03:07
If the initial number of elements is odd, the code even has undefined behavior. Good that it's so easy to fix. :-) – Arne Vogel Feb 08 '18 at 10:02

AndreyS Scherbakov · Answer 2 · 2018-02-08T03:05:38.883

0

It's quite enough to remember the next iterator value before doing any deletion/insertion at the current position. This will help you to keep yourself careless about what and when are your modifications concretely doing.

list<int>::iterator it = numbers.begin();
while (it != numbers.end()) {
{
list<int>::iterator next_it = it;
++next_it;

// Do your insertion or deletion here

it = next_it;
}

edited Feb 08 '18 at 03:05

answered Feb 08 '18 at 02:57

AndreyS Scherbakov

2,674
2
20
27

Calling erase() on an iterator, but not deleting correct element

2 Answers2