How to remove from a list inside a for loop in c++

Question

Is there any way to remove a list item inside a for loop that loops through that list? example:

std::list<int> myList;
myList.push_back(5);
myList.push_back(8);

std::list<int>::iterator i;

for (i = myList.begin(); i != myList.end(); i++)
{
    if (i == 8)
        // myList.remove(*i);
}

Is there any way to replace the myList.remove(*i) with something else, because that gives an error.

*Is there any way to remove a list item inside a for loop that loops through that list?* -- What is it that you really want to do? Remove all items from the list that match a certain criteria? You don't need a loop to do this. — PaulMcKenzie, Jul 19 '21 at 05:56
`i` is iterator, eg pointer, so you I'd like to dereference to compare values `if (*i == 8)...` — user3124812, Jul 19 '21 at 05:56
@SanduChicu The reason why I asked the question is that this is looking more like an [XY Problem](https://xyproblem.info/). You are giving your "solution", but not mentioning the actual problem. The solution is not to write a loop. — PaulMcKenzie, Jul 19 '21 at 06:07
@PaulMcKenzie's answer is the best and must be voted up on the top for future askers. — 273K, Jul 19 '21 at 06:19

score 5 · Answer 1 · answered Jul 19 '21 at 06:08

To erase all items that equal 8, simply use the erase/remove idiom. There is no need to write any loops:

#include <list>
#include <algorithm>
#include <iostream>

int main()
{
    std::list<int> myList;
    myList.push_back(5);
    myList.push_back(8);
    std::cout << "Before:\n";
    for (auto i : myList)
       std::cout << i << "\n";

    // Erase all the items that equal 8
    myList.erase(std::remove(myList.begin(), myList.end(), 8), myList.end());    

    std::cout << "\nAfter:\n";
    for (auto i : myList)
       std::cout << i << "\n";
}

Output:

Before:
5
8

After:
5

This works, and has the benefit of making it interchangable with vector (not sure about other containers). Though for list only using erase-remove seems superfluous. — alagner, Jul 19 '21 at 06:15

foragerDev · Answer 2 · 2021-07-19T05:58:36.050

3

You are using iterator so there is a method erase. Which you can use like this

while (i != myList.end())
{
    if (*i == 8) // dereferance the i
       i = myList.erase(i);
    else i++;
}

edited Jul 19 '21 at 05:58

answered Jul 19 '21 at 05:48

foragerDev

1,307
1
9
22

1

Your loop will (potentially) increment erased iterator. – alagner Jul 19 '21 at 05:49
so do you know how to fix that? im kinda new to c++ – Sandu Chicu Jul 19 '21 at 05:54
2

You sure you don't want `i = myList.erase(i);`? https://en.cppreference.com/w/cpp/container/list/erase – Retired Ninja Jul 19 '21 at 05:57
there is one more problem: if there's only one item in the list, it will show this error: "Expression: cannot increment end list iterator" – Sandu Chicu Jul 19 '21 at 06:03

alagner · Accepted Answer · 2021-07-19T06:16:33.607

0

First and foremost: if iterating over the whole list just to delete the desired items, just use list::remove without the loop. It will do the trick for you.

However when the loop is needed for this or other reason list::erase is the way to go, however it will require manual iterator adjustments: See: https://stackoverflow.com/a/596180/4885321 Regular for loop will not work as expected, because (bad code ahead):

for(auto i = l.begin();
  i!=l.end();
  ++i) //2nd iteration starts, we're incrementing non-existing iterator, UB!
{
  if((*i) == myVal) //let's assume it's true for the first element
    l.erase(i);  //ok, erase it
}

Thus, the right solution should look like this:

while (i != l.end()) {
    if (*i == myVal) {
        l.erase(i++);
        // or i = l.erase(i);
        // note: valid iterator is incremented before call to erase thus the new i is valid
        // the node pointed to by the old one is removed
    } else {
        ++i;
    }
}

I recommend looking up in Meyers's Effective STL for further information on that and related topics.

edited Jul 19 '21 at 06:16

answered Jul 19 '21 at 06:03

alagner

3,448
1
13
25

I personally would still prefer a `for` loop for limiting the scope of `i` to exactly this loop (even though not done in the question either): `for(auto i = l.begin(); i != l.end();)` with empty expression being executed after the loop. – Aconcagua Jul 19 '21 at 06:36
@SanduChicu Please note that this algorithm is fine for `std::list` as in your question, if you ever switch to `std::vector` it will be inefficient as with every erase all subsequent elements are copied one position towards front. In *that* case you'll need *another* algorithm. – Aconcagua Jul 19 '21 at 06:38
@Aconcagua For sure your suggestion is also correct, but to me a for loop should have its own incrementation instruction, and I wanted to avoid monstrosities like that `for (auto i = l.begin(), e=l.end(); i!=e; i = (*i==myVal) ? l.erase(i) : next(i));` But hard to argue on style, especially in an out-of-context demo code. – alagner Jul 19 '21 at 06:46

How to remove from a list inside a for loop in c++

3 Answers3