Удаление элементов из набора STL во время итерации - PullRequest
126 голосов
/ 20 мая 2010

Мне нужно пройти через набор и удалить элементы, которые соответствуют предопределенным критериям.

Это тестовый код, который я написал:

#include <set>
#include <algorithm>

void printElement(int value) {
    std::cout << value << " ";
}

int main() {
    int initNum[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
    std::set<int> numbers(initNum, initNum + 10);
    // print '0 1 2 3 4 5 6 7 8 9'
    std::for_each(numbers.begin(), numbers.end(), printElement);

    std::set<int>::iterator it = numbers.begin();

    // iterate through the set and erase all even numbers
    for (; it != numbers.end(); ++it) {
        int n = *it;
        if (n % 2 == 0) {
            // wouldn't invalidate the iterator?
            numbers.erase(it);
        }
    }

    // print '1 3 5 7 9'
    std::for_each(numbers.begin(), numbers.end(), printElement);

    return 0;
}

Сначала я подумал, что удаление элемента из набора во время его итерации приведет к аннулированию итератора, и приращение в цикле for будет иметь неопределенное поведение. Хотя я выполнил этот тестовый код, и все прошло хорошо, и я не могу объяснить, почему.

Мой вопрос: Это определенное поведение для стандартных наборов или это конкретная реализация? Кстати, я использую gcc 4.3.3 в Ubuntu 10.04 (32-битная версия).

Спасибо!

Предлагаемое решение:

Это правильный способ повторять и удалять элементы из набора?

while(it != numbers.end()) {
    int n = *it;
    if (n % 2 == 0) {
        // post-increment operator returns a copy, then increment
        numbers.erase(it++);
    } else {
        // pre-increment operator increments, then return
        ++it;
    }
}

Редактировать: ПРЕДПОЧТИТЕЛЬНОЕ РЕШЕНИЕ

Я нашел решение, которое кажется мне более элегантным, хотя оно и делает то же самое.

while(it != numbers.end()) {
    // copy the current iterator then increment it
    std::set<int>::iterator current = it++;
    int n = *current;
    if (n % 2 == 0) {
        // don't invalidate iterator it, because it is already
        // pointing to the next element
        numbers.erase(current);
    }
}

Если в то время как есть несколько условий тестирования, каждое из них должно увеличивать итератор. Мне больше нравится этот код, потому что итератор увеличивается только в одном месте , делая код менее подверженным ошибкам и более читабельным.

Ответы [ 8 ]

156 голосов
/ 20 мая 2010

Это зависит от реализации:

Стандарт 23.1.2.8:

Элементы вставки не должны влиять на действительность итераторов и ссылок на контейнер, а члены стирания должны делать недействительными только итераторы и ссылки на стертые элементы.

Может быть, вы могли бы попробовать это - это стандартное соответствие:

for (auto it = numbers.begin(); it != numbers.end(); ) {
    if (*it % 2 == 0) {
        numbers.erase(it++);
    }
    else {
        ++it;
    }
}

Обратите внимание, что это ++ является постфиксом, поэтому он передает старую позицию для удаления, но сначала переходит к новой позиции из-за оператора.

2015.10.27 обновление: C ++ 11 устранил дефект. iterator erase (const_iterator position); возвращает итератор для элемента, следующего за последним удаленным элементом (или set::end, если последний элемент был удален). Итак, стиль C ++ 11:

for (auto it = numbers.begin(); it != numbers.end(); ) {
    if (*it % 2 == 0) {
        it = numbers.erase(it);
    }
    else {
        ++it;
    }
}
18 голосов
/ 20 мая 2010

Если вы запустите свою программу через valgrind, вы увидите кучу ошибок чтения. Другими словами, да, итераторы становятся недействительными, но вам повезло в вашем примере (или вам действительно не повезло, поскольку вы не видите негативных последствий неопределенного поведения). Одним из решений этого является создание временного итератора, увеличение временного значения, удаление целевого итератора, а затем установка целевого значения для временного. Например, переписать ваш цикл следующим образом:

std::set<int>::iterator it = numbers.begin();                               
std::set<int>::iterator tmp;                                                

// iterate through the set and erase all even numbers                       
for ( ; it != numbers.end(); )                                              
{                                                                           
    int n = *it;                                                            
    if (n % 2 == 0)                                                         
    {                                                                       
        tmp = it;                                                           
        ++tmp;                                                              
        numbers.erase(it);                                                  
        it = tmp;                                                           
    }                                                                       
    else                                                                    
    {                                                                       
        ++it;                                                               
    }                                                                       
} 
7 голосов
/ 20 мая 2010

Вы неправильно понимаете, что означает "неопределенное поведение". Неопределенное поведение не означает «если вы сделаете это, ваша программа будет аварийно завершать работу или давать неожиданные результаты». Это означает, что «если вы сделаете это, ваша программа может аварийно завершить работу или привести к неожиданным результатам», или сделать что-либо еще, в зависимости от вашего компилятора, вашей операционной системы, фазы луны и т. Д.

Если что-то выполняется без сбоев и ведет себя так, как вы этого ожидаете, то это не доказательство того, что это не неопределенное поведение. Все, что он доказывает, это то, что его поведение оказалось таким же, как наблюдалось для этого конкретного прогона после компиляции с этим конкретным компилятором в этой конкретной операционной системе.

Стирание элемента из набора делает итератор стертым элементом недействительным. Использование недействительного итератора - неопределенное поведение. Так уж случилось, что наблюдаемое поведение было тем, что вы намеревались в данном конкретном случае; это не значит, что код правильный.

2 голосов
/ 23 августа 2015

Просто чтобы предупредить, что в случае контейнера deque все решения, которые проверяют равенство итератора deque для numbers.end (), вероятно, потерпят неудачу на gcc 4.8.4. А именно, удаление элемента deque обычно делает недействительным указатель на numbers.end ():

#include <iostream>
#include <deque>

using namespace std;
int main() 
{

  deque<int> numbers;

  numbers.push_back(0);
  numbers.push_back(1);
  numbers.push_back(2);
  numbers.push_back(3);
  //numbers.push_back(4);

  deque<int>::iterator  it_end = numbers.end();

  for (deque<int>::iterator it = numbers.begin(); it != numbers.end(); ) {
    if (*it % 2 == 0) {
      cout << "Erasing element: " << *it << "\n";
      numbers.erase(it++);
      if (it_end == numbers.end()) {
    cout << "it_end is still pointing to numbers.end()\n";
      } else {
    cout << "it_end is not anymore pointing to numbers.end()\n";
      }
    }
    else {
      cout << "Skipping element: " << *it << "\n";
      ++it;
    }
  }
}

Выход:

Erasing element: 0
it_end is still pointing to numbers.end()
Skipping element: 1
Erasing element: 2
it_end is not anymore pointing to numbers.end()

Обратите внимание, что, хотя преобразование deque корректно в этом конкретном случае, указатель конца был недействительным на этом пути. С deque другого размера ошибка более очевидна:

int main() 
{

  deque<int> numbers;

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

  deque<int>::iterator  it_end = numbers.end();

  for (deque<int>::iterator it = numbers.begin(); it != numbers.end(); ) {
    if (*it % 2 == 0) {
      cout << "Erasing element: " << *it << "\n";
      numbers.erase(it++);
      if (it_end == numbers.end()) {
    cout << "it_end is still pointing to numbers.end()\n";
      } else {
    cout << "it_end is not anymore pointing to numbers.end()\n";
      }
    }
    else {
      cout << "Skipping element: " << *it << "\n";
      ++it;
    }
  }
}

Выход:

Erasing element: 0
it_end is still pointing to numbers.end()
Skipping element: 1
Erasing element: 2
it_end is still pointing to numbers.end()
Skipping element: 3
Erasing element: 4
it_end is not anymore pointing to numbers.end()
Erasing element: 0
it_end is not anymore pointing to numbers.end()
Erasing element: 0
it_end is not anymore pointing to numbers.end()
...
Segmentation fault (core dumped)

Вот один из способов исправить это:

#include <iostream>
#include <deque>

using namespace std;
int main() 
{

  deque<int> numbers;
  bool done_iterating = false;

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

  if (!numbers.empty()) {
    deque<int>::iterator it = numbers.begin();
    while (!done_iterating) {
      if (it + 1 == numbers.end()) {
    done_iterating = true;
      } 
      if (*it % 2 == 0) {
    cout << "Erasing element: " << *it << "\n";
      numbers.erase(it++);
      }
      else {
    cout << "Skipping element: " << *it << "\n";
    ++it;
      }
    }
  }
}
1 голос
/ 10 января 2019

C ++ 20 будет иметь «равномерное стирание контейнера», и вы сможете написать:

std::erase_if(numbers, [](int n){ return n % 2 == 0 });

И это будет работать для vector, set, deque,и т.д. См. cppReference для получения дополнительной информации.

1 голос
/ 10 января 2019

Я думаю, что использование метода STL 'remove_if' из может помочь предотвратить некоторые странные проблемы при попытке удаления объекта, обернутого итератором.

Это решение может быть менее эффективным.

Допустим, у нас есть какой-то контейнер, например, vector или список с именем m_bullets:

Bullet::Ptr is a shared_pr<Bullet>

'it' - это итератор, который возвращает 'remove_if', третий аргумент - это лямбда-функция, которая выполняется для каждого элемента контейнера. Поскольку контейнер содержит Bullet::Ptr, лямбда-функция должна получить этот тип (или ссылку на этот тип) в качестве аргумента.

 auto it = std::remove_if(m_bullets.begin(), m_bullets.end(), [](Bullet::Ptr bullet){
    // dead bullets need to be removed from the container
    if (!bullet->isAlive()) {
        // lambda function returns true, thus this element is 'removed'
        return true;
    }
    else{
        // in the other case, that the bullet is still alive and we can do
        // stuff with it, like rendering and what not.
        bullet->render(); // while checking, we do render work at the same time
        // then we could either do another check or directly say that we don't
        // want the bullet to be removed.
        return false;
    }
});
// The interesting part is, that all of those objects were not really
// completely removed, as the space of the deleted objects does still 
// exist and needs to be removed if you do not want to manually fill it later 
// on with any other objects.
// erase dead bullets
m_bullets.erase(it, m_bullets.end());

'remove_if' удаляет контейнер, в котором лямбда-функция вернула true, и переносит это содержимое в начало контейнера. 'it' указывает на неопределенный объект, который можно считать мусором. Объекты от 'it' до m_bullets.end () могут быть удалены, так как они занимают память, но содержат мусор, поэтому в этом диапазоне вызывается метод 'erase'.

1 голос
/ 20 мая 2010

Это поведение зависит от реализации. Чтобы гарантировать правильность итератора, вы должны использовать «it = numbers.erase (it);» заявление, если вам нужно удалить элемент и просто итератор incerement в другом случае.

0 голосов
/ 02 июля 2018

Я столкнулся с той же самой старой проблемой и нашел ниже код более понятный , что в некотором смысле соответствует приведенным выше решениям.

std::set<int*>::iterator beginIt = listOfInts.begin();
while(beginIt != listOfInts.end())
{
    // Use your member
    std::cout<<(*beginIt)<<std::endl;

    // delete the object
    delete (*beginIt);

    // erase item from vector
    listOfInts.erase(beginIt );

    // re-calculate the begin
    beginIt = listOfInts.begin();
}
...