Você pode remover elementos de uma lista std :: enquanto itera através dela?

239

Eu tenho um código que se parece com isso:

for (std::list<item*>::iterator i=items.begin();i!=items.end();i++)
{
    bool isActive = (*i)->update();
    //if (!isActive) 
    //  items.remove(*i); 
    //else
       other_code_involving(*i);
}
items.remove_if(CheckItemNotActive);

Gostaria de remover itens inativos imediatamente após atualizá-los, para evitar andar na lista novamente. Mas se eu adicionar as linhas comentadas, recebo um erro quando chego a i++: "Listador de iterador não incremental". Tentei algumas alternativas que não foram incrementadas na instrução for, mas não consegui fazer nada funcionar.

Qual é a melhor maneira de remover itens enquanto você caminha por uma lista std ::?

AShelly
fonte
Não vi nenhuma solução baseada na iteração reversa. Eu postei um desses .
sancho.s ReinstateMonicaCellio

Respostas:

286

Você deve incrementar o iterador primeiro (com i ++) e depois remover o elemento anterior (por exemplo, usando o valor retornado de i ++). Você pode alterar o código para um loop while da seguinte maneira:

std::list<item*>::iterator i = items.begin();
while (i != items.end())
{
    bool isActive = (*i)->update();
    if (!isActive)
    {
        items.erase(i++);  // alternatively, i = items.erase(i);
    }
    else
    {
        other_code_involving(*i);
        ++i;
    }
}
Michael Kristofik
fonte
7
Na verdade, isso não garante que funcione. Com "apagar (i ++);", sabemos apenas que o valor pré-incrementado é passado para apagar (), e o i é incrementado antes do ponto e vírgula, não necessariamente antes da chamada para apagar (). "iterador prev = i ++; apagar (prev);" é certo para o trabalho, como é usar o valor de retorno
James Curran
58
No James, i é incrementado antes de chamar apagar, e o valor anterior é passado para a função. Os argumentos de uma função devem ser totalmente avaliados antes que a função seja chamada.
27711 Brian Neal
28
@ James Curran: Isso não está correto. Todos os argumentos são totalmente avaliados antes que uma função seja chamada.
287 Martin Martin
9
Martin York está correto. Todos os argumentos para uma chamada de função são totalmente avaliados antes que uma função seja chamada, sem exceções. É assim que a função funciona. E não tem nada a ver com o seu foo.b (i ++) c (i ++) exemplo (que é indefinido em qualquer caso).
jalf
75
O uso alternativo i = items.erase(i)é mais seguro, porque é equivalente a uma lista, mas ainda funcionará se alguém alterar o contêiner para um vetor. Com um vetor, apague () move tudo para a esquerda para preencher o buraco. Se você tentar remover o último item com um código que incrementa o iterador após a exclusão, o final se move para a esquerda e o iterador se move para a direita - além do final. E então você bate.
Eric Seppanen
133

Você quer fazer:

i= items.erase(i);

Isso atualizará corretamente o iterador para apontar para o local após o iterador que você removeu.

MSN
fonte
80
Esteja avisado de que você não pode simplesmente soltar esse código em seu loop for. Caso contrário, você pulará um elemento toda vez que o remover.
22430 Michael Kristofik
2
ele não pode fazer eu--; cada vez que segue seu código para evitar pular?
enthusiasticgeek
1
@enthusiasticgeek, o que acontece se i==items.begin()?
MSN
1
@enthusiasticgeek, nesse ponto você deveria fazer i= items.erase(i);. É a forma canônica e já cuida de todos esses detalhes.
MSN
6
Michael aponta uma enorme pegadinha, eu tive que lidar com a mesma coisa agora. Método mais fácil de evitar que eu encontrei foi apenas decompondo o loop for () em um while () e tomando cuidado com o incremento
Anne Quinn
22

Você precisa fazer a combinação da resposta do Kristo e do MSN:

// Note: Using the pre-increment operator is preferred for iterators because
//       there can be a performance gain.
//
// Note: As long as you are iterating from beginning to end, without inserting
//       along the way you can safely save end once; otherwise get it at the
//       top of each loop.

std::list< item * >::iterator iter = items.begin();
std::list< item * >::iterator end  = items.end();

while (iter != end)
{
    item * pItem = *iter;

    if (pItem->update() == true)
    {
        other_code_involving(pItem);
        ++iter;
    }
    else
    {
        // BTW, who is deleting pItem, a.k.a. (*iter)?
        iter = items.erase(iter);
    }
}

Obviamente, a coisa mais eficiente e mais eficiente do SuperCool® STL seria algo como isto:

// This implementation of update executes other_code_involving(Item *) if
// this instance needs updating.
//
// This method returns true if this still needs future updates.
//
bool Item::update(void)
{
    if (m_needsUpdates == true)
    {
        m_needsUpdates = other_code_involving(this);
    }

    return (m_needsUpdates);
}

// This call does everything the previous loop did!!! (Including the fact
// that it isn't deleting the items that are erased!)
items.remove_if(std::not1(std::mem_fun(&Item::update)));
Mike
fonte
Eu considerei o seu método SuperCool, minha hesitação foi que a chamada para remove_if não deixa claro que o objetivo é processar os itens, em vez de removê-los da lista de ativos. (Os itens não estão sendo excluídas porque eles só estão se tornando inativo, não desnecessários)
AShelly
Suponho que esteja certo. Por um lado, estou inclinado a sugerir alterar o nome de 'update' para remover a obscuridade, mas a verdade é que esse código é sofisticado com os functores, mas também é tudo menos obscuro.
236 Mike Mike
Comentário justo, corrija o loop while para usar end ou exclua a definição não utilizada.
Mike
10

Use o algoritmo std :: remove_if.

Editar: O trabalho com coleções deve ser como: 1. preparar a coleção. 2. coleção de processos.

A vida será mais fácil se você não misturar essas etapas.

  1. std :: remove_if. ou list :: remove_if (se você sabe que trabalha com list e não com o TCollection)
  2. std :: for_each
Mykola Golubyev
fonte
2
std :: list possui uma função de membro remove_if, que é mais eficiente que o algoritmo remove_if (e não requer o idioma "remove-erase").
27711 Brian Neal
5

Aqui está um exemplo usando um forloop que itera a lista e incrementa ou revalida o iterador no caso de um item ser removido durante a travessia da lista.

for(auto i = items.begin(); i != items.end();)
{
    if(bool isActive = (*i)->update())
    {
        other_code_involving(*i);
        ++i;

    }
    else
    {
        i = items.erase(i);

    }

}

items.remove_if(CheckItemNotActive);
David Cormack
fonte
4

A alternativa para a versão em loop da resposta de Kristo.

Você perde alguma eficiência, retrocede e avança novamente ao excluir, mas em troca do incremento extra do iterador, o iterador é declarado no escopo do loop e o código fica um pouco mais limpo. O que escolher depende das prioridades do momento.

A resposta estava totalmente sem tempo, eu sei ...

typedef std::list<item*>::iterator item_iterator;

for(item_iterator i = items.begin(); i != items.end(); ++i)
{
    bool isActive = (*i)->update();

    if (!isActive)
    {
        items.erase(i--); 
    }
    else
    {
        other_code_involving(*i);
    }
}
Rafael Gago
fonte
1
É o que eu usei também. Mas não tenho certeza se é garantido que funcione se o elemento a ser removido for o primeiro elemento no contêiner. Para mim, funciona, eu acho, mas não tenho certeza se é portátil entre plataformas.
trololo
Eu não fiz "-1", no entanto, o iterador da lista não pode diminuir? Pelo menos eu tenho afirmação do Visual Studio 2008.
milesma
Contanto que a lista vinculada seja implementada como uma lista circular dupla vinculada com um nó head / stub (usado como end () rbegin () e quando vazio usado como begin () e rend () também), isso funcionará. Não me lembro em qual plataforma estava usando isso, mas também estava funcionando para mim, pois a implementação mencionada acima é a implementação mais comum para uma lista std ::. De qualquer forma, é quase certo que isso esteja explorando algum comportamento indefinido (pelo padrão C ++), então é melhor não usá-lo.
Rafael Gago
re: iterator cannot be decrementedO erasemétodo precisa de um random access iterator. Algumas implementações de coleção fornecem um forward only iteratorque causa a afirmação.
Jesse Chisholm
@ Jessie Chisholm, a pergunta era sobre uma lista std ::, não um contêiner arbitrário. std :: list fornece apagadores e iteradores bidirecionais.
Rafael Gago
4

Eu tenho resumo, aqui está o método três com o exemplo:

1. usando whileloop

list<int> lst{4, 1, 2, 3, 5};

auto it = lst.begin();
while (it != lst.end()){
    if((*it % 2) == 1){
        it = lst.erase(it);// erase and go to next
    } else{
        ++it;  // go to next
    }
}

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2

2. usando a remove_iffunção de membro na lista:

list<int> lst{4, 1, 2, 3, 5};

lst.remove_if([](int a){return a % 2 == 1;});

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2

3. usando a std::remove_iffunção combinada com a erasefunção membro:

list<int> lst{4, 1, 2, 3, 5};

lst.erase(std::remove_if(lst.begin(), lst.end(), [](int a){
    return a % 2 == 1;
}), lst.end());

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2

4. Usando forloop, observe o update do iterador:

list<int> lst{4, 1, 2, 3, 5};

for(auto it = lst.begin(); it != lst.end();++it){
    if ((*it % 2) == 1){
        it = lst.erase(it);  erase and go to next(erase will return the next iterator)
        --it;  // as it will be add again in for, so we go back one step
    }
}

for(auto it:lst)cout<<it<<" ";
cout<<endl;  //4 2 
Jayhello
fonte
2

A remoção invalida apenas os iteradores que apontam para os elementos removidos.

Portanto, neste caso, após remover * i, i é invalidado e você não pode incrementá-lo.

O que você pode fazer é primeiro salvar o iterador do elemento a ser removido, depois incrementar o iterador e depois remover o salvo.

anand
fonte
2
Usar pós-incremento é muito mais elegante.
27711 Brian Neal
2

Se você pensar na std::listfila como uma fila, poderá desenfileirar e enfileirar todos os itens que deseja manter, mas desenfileirar apenas (e não enfileirar) o item que deseja remover. Aqui está um exemplo em que quero remover 5 de uma lista que contém os números de 1 a 10 ...

std::list<int> myList;

int size = myList.size(); // The size needs to be saved to iterate through the whole thing

for (int i = 0; i < size; ++i)
{
    int val = myList.back()
    myList.pop_back() // dequeue
    if (val != 5)
    {
         myList.push_front(val) // enqueue if not 5
    }
}

myList agora só terá os números 1-4 e 6-10.

Alex Bagg
fonte
Abordagem interessante, mas receio que possa ser lento.
SG7
2

A iteração para trás evita o efeito de apagar um elemento nos elementos restantes a serem atravessados:

typedef list<item*> list_t;
for ( list_t::iterator it = items.end() ; it != items.begin() ; ) {
    --it;
    bool remove = <determine whether to remove>
    if ( remove ) {
        items.erase( it );
    }
}

PS: veja isto , por exemplo, com relação à iteração reversa.

PS2: Eu não testei completamente se ele lida bem com elementos de apagamento nas extremidades.

sancho.s ReinstateMonicaCellio
fonte
re: avoids the effect of erasing an element on the remaining elementspara uma lista, provavelmente sim. Para um vetor talvez não. Isso não é algo garantido em coleções arbitrárias. Por exemplo, um mapa pode decidir se reequilibrar.
Jesse Chisholm
1

Você pode escrever

std::list<item*>::iterator i = items.begin();
while (i != items.end())
{
    bool isActive = (*i)->update();
    if (!isActive) {
        i = items.erase(i); 
    } else {
        other_code_involving(*i);
        i++;
    }
}

Você pode escrever um código equivalente com std::list::remove_if, que é menos detalhado e mais explícito

items.remove_if([] (item*i) {
    bool isActive = (*i)->update();
    if (!isActive) 
        return true;

    other_code_involving(*i);
    return false;
});

O std::vector::erase std::remove_ifidioma deve ser usado quando itens é um vetor em vez de uma lista para manter a compexidade em O (n) - ou caso você escreva código genérico e os itens possam ser um contêiner sem uma maneira eficaz de apagar itens únicos (como um vetor)

items.erase(std::remove_if(begin(items), end(items), [] (item*i) {
    bool isActive = (*i)->update();
    if (!isActive) 
        return true;

    other_code_involving(*i);
    return false;
}));
J. Kalz
fonte
-4

Eu acho que você tem um bug lá, eu codigo desta maneira:

for (std::list<CAudioChannel *>::iterator itAudioChannel = audioChannels.begin();
             itAudioChannel != audioChannels.end(); )
{
    CAudioChannel *audioChannel = *itAudioChannel;
    std::list<CAudioChannel *>::iterator itCurrentAudioChannel = itAudioChannel;
    itAudioChannel++;

    if (audioChannel->destroyMe)
    {
        audioChannels.erase(itCurrentAudioChannel);
        delete audioChannel;
        continue;
    }
    audioChannel->Mix(outBuffer, numSamples);
}
Marcin Skoczylas
fonte
Eu estou supondo que isso foi votado para preferências de estilo, pois parece ser funcional. Sim, claro, (1) ele usa um iterador extra, (2) o incremento do iterador está em um local estranho para um loop sem uma boa razão para colocá-lo lá, (3) ele faz o canal funcionar após a decisão de excluir de antes, como no OP. Mas não é uma resposta errada.
Jesse Chisholm