Удаление элементов из std :: set во время итерации

147

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

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

#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);
    }
}

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

pedromanoel
источник
1
На вопрос и ответ: stackoverflow.com/questions/263945/…
Мартин Йорк
3
На самом деле, я прочитал этот (и другие) вопрос перед тем, как задавать мой, но поскольку они были связаны с другими контейнерами STL и поскольку мой первоначальный тест, очевидно, работал, я подумал, что между ними есть некоторая разница. Только после ответа Мэтта я подумал об использовании valgrind. Несмотря на то, что я предпочитаю свое НОВОЕ решение над другими, потому что оно уменьшает вероятность ошибок, увеличивая итератор только в одном месте. Спасибо всем за помощь!
Педроманоэль
1
@pedromanoel ++itдолжен быть несколько более эффективным, чем it++потому, что он не требует использования невидимой временной копии итератора. Версия Корнеля, хотя и дольше гарантирует, что нефильтрованные элементы будут проходить итерацию наиболее эффективно.
Альнитак
@Alnitak Я не думал об этом, но думаю, что разница в производительности не будет такой большой. Копия также создается в его версии, но только для соответствующих элементов. Так что степень оптимизации полностью зависит от структуры множества. В течение некоторого времени я предварительно оптимизировал код, снижая читабельность и скорость кодирования в процессе ... Поэтому я бы выполнил несколько тестов, прежде чем использовать другой способ.
Pedromanoel

Ответы:

178

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

Стандарт 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;
    }
}
Корнел Киселевич
источник
2
Это не работает с deque MSVC2013. Либо их реализация содержит ошибки, либо существует еще одно требование, которое мешает этому работать deque. Спецификация STL настолько сложна, что вы не можете ожидать, что все реализации будут следовать ей, не говоря уже о том, что ваш программист запомнит ее. STL - чудовище, за пределами укрощения, и поскольку нет уникальной реализации (и тестовые наборы, если таковые имеются, очевидно, не охватывают такие очевидные случаи, как удаление элементов в цикле), что делает STL блестящей хрупкой игрушкой, которая может подняться удар, когда вы смотрите на него вбок.
kuroi neko
@MatthieuM. Это в C ++ 11. В C ++ 17 теперь требуется итератор (const_iterator в C ++ 11).
tartaruga_casco_mole
19

Если вы запустите свою программу через 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;                                                               
    }                                                                       
} 
Matt
источник
Если это единственное условие, которое имеет значение и не требует инициализации в объеме или последующей операции, то лучше использовать whileцикл. то for ( ; it != numbers.end(); )есть лучше видно сwhile (it != numbers.end())
iammilind
7

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

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

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

Тайлер МакГенри
источник
О, я хорошо знаю, что неопределенное поведение может также означать «Это работает для меня, но не для всех». Вот почему я задал этот вопрос, потому что я не знал, было ли это поведение правильным или нет. Если бы это было так, я бы просто так ушел. Использование цикла while решит мою проблему, тогда? Я отредактировал свой вопрос предложенным решением. Пожалуйста, проверьте это.
педроманоэль
У меня это тоже работает. Но когда я изменяю условие на if (n > 2 && n < 7 )тогда, я получаю 0 1 2 4 7 8 9. - Конкретный результат здесь, вероятно, больше зависит от деталей реализации метода стирания и итераторов набора, а не от фазы луны (не той следует когда-либо полагаться на детали реализации). ;)
UncleBens
1
STL добавляет много нового значения в «неопределенное поведение». Например, «Microsoft считала целесообразным усовершенствовать спецификацию, позволяя std::set::eraseвозвращать итератор, чтобы ваш код MSVC работал с треском при компиляции с помощью gcc», или «Microsoft выполняет привязанные проверки, std::bitset::operator[]поэтому ваш тщательно оптимизированный алгоритм набора битов замедлится до сканировать при компиляции с MSVC ". У STL нет уникальной реализации, и его спецификация - экспоненциально растущий раздутый беспорядок, поэтому неудивительно, что удаление элементов из цикла требует опыта старшего программиста ...
kuroi neko
2

Просто чтобы предупредить, что в случае контейнера 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;
      }
    }
  }
}
McKryak
источник
Ключевое существо do not trust an old remembered dq.end() value, always compare to a new call to dq.end().
Джесси Чисхолм
2

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

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

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

Маршалл Клоу
источник
1

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

Виталий Богданов
источник
1
Set<T>::eraseверсия не возвращает итератор.
Аркаитц Хименес
4
На самом деле это так, но только на реализацию MSVC. Так что это действительно конкретный ответ для реализации. :)
Евгений
1
@Eugene Делает это для всех реализаций с C ++ 11
мастов
В некоторых реализациях gcc 4.8с c++1yошибкой стереть. it = collection.erase(it);должен работать, но это может быть более безопасно для использованияcollection.erase(it++);
Джесси Чисхолм
1

Я думаю, используя метод STLremove_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' удаляет контейнер, в котором лямбда-функция вернула истину, и перемещает это содержимое в начало контейнера. Символ ' it' указывает на неопределенный объект, который можно считать мусором. Объекты от 'it' до m_bullets.end () могут быть удалены, так как они занимают память, но содержат мусор, поэтому в этом диапазоне вызывается метод 'erase'.

Джон Бем
источник
0

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

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();
}
Анураг
источник
Это работает, только если вы всегда будете стирать каждый элемент. OP предназначен для выборочного удаления элементов и сохранения действительных итераторов.
Джесси Чисхолм