Removendo item do vetor, enquanto no intervalo 'for' do C ++ 11?

97

Tenho um vetor de IInventory * e estou percorrendo a lista usando o intervalo C ++ 11 para, para fazer coisas com cada um.

Depois de fazer algumas coisas com um, posso querer removê-lo da lista e excluir o objeto. Sei que posso chamar deleteo ponteiro a qualquer momento para limpá-lo, mas qual é a maneira correta de removê-lo do vetor, enquanto está no forloop de intervalo ? E se eu removê-lo da lista, meu loop será invalidado?

std::vector<IInventory*> inv;
inv.push_back(new Foo());
inv.push_back(new Bar());

for (IInventory* index : inv)
{
    // Do some stuff
    // OK, I decided I need to remove this object from 'inv'...
}
EddieV223
fonte
8
Se você quiser ser mais sofisticado, poderá usar std::remove_ifcom um predicado que "faz coisas" e retornar true se quiser que o elemento seja removido.
Benjamin Lindley
Existe uma razão pela qual você não pode simplesmente adicionar um contador de índice e, em seguida, usar algo como inv.erase (índice)?
TomJ
4
@TomJ: Isso ainda atrapalharia a iteração.
Ben Voigt
@TomJ e seria um assassino de desempenho - em cada apagamento, você consegue mover todos os elementos após o apagado.
Ivan,
1
@BenVoigt Eu recomendei mudar para std::listabaixo
bobobobo

Respostas:

94

Não, você não pode. Baseado em intervalo foré para quando você precisa acessar cada elemento de um contêiner uma vez.

Você deve usar o forloop normal ou um de seus primos se precisar modificar o contêiner à medida que avança, acessar um elemento mais de uma vez ou, de outra forma, iterar de forma não linear por meio do contêiner.

Por exemplo:

auto i = std::begin(inv);

while (i != std::end(inv)) {
    // Do some stuff
    if (blah)
        i = inv.erase(i);
    else
        ++i;
}
Seth Carnegie
fonte
5
Não seria o idioma apagar-remover aplicável aqui?
Naveen
4
@Naveen Decidi não tentar fazer isso porque aparentemente ele precisa iterar cada item, fazer cálculos com ele e, em seguida, possivelmente removê-lo do contêiner. Apagar-remover diz que você apenas apaga os elementos para os quais um predicado retorna true, AFAIU, e parece melhor dessa forma não misturar a lógica de iteração com o predicado.
Seth Carnegie
4
@SethCarnegie Erase-remove com um lambda para o predicado elegantemente permite que (uma vez que este já é C ++ 11)
Potatoswatter
11
Não gosto desta solução, é O (N ^ 2) para a maioria dos contêineres. remove_ifé melhor.
Ben Voigt
6
esta resposta está correta, eraseretorna um novo iterador válido. pode não ser eficiente, mas funciona com certeza.
sp2danny
56

Cada vez que um elemento é removido do vetor, você deve assumir que os iteradores no momento ou após o elemento apagado não são mais válidos, porque cada um dos elementos que sucedem o elemento apagado são movidos.

Um for-loop baseado em intervalo é apenas açúcar sintático para loop "normal" usando iteradores, portanto, o acima se aplica.

Dito isso, você pode simplesmente:

inv.erase(
    std::remove_if(
        inv.begin(),
        inv.end(),
        [](IInventory* element) -> bool {
            // Do "some stuff", then return true if element should be removed.
            return true;
        }
    ),
    inv.end()
);
Branko Dimitrijevic
fonte
5
" porque existe a possibilidade de que o vetor realoque o bloco de memória no qual mantém seus elementos " Não, a vectornunca será realocado devido a uma chamada a erase. A razão pela qual os iteradores são invalidados é porque cada um dos elementos que sucedem ao elemento apagado é movido.
ildjarn
2
Um padrão de captura de [&]seria apropriado, para permitir que ele "faça algumas coisas" com variáveis ​​locais.
Potatoswatter
2
Isso não parece mais simples do que um loop baseada iterador, além disso você tem que lembrar de cercar o seu remove_ifcom .erase, caso contrário, nada acontece.
bobobobo
2
@bobobobo Se por "loop baseado em iterador" você quer dizer a resposta de Seth Carnegie , isso é O (n ^ 2) em média. std::remove_ifé O (n).
Branko Dimitrijevic
Você acertou em cheio , trocando os elementos no final da lista e evitando realmente mover os elementos até que todos os elementos "a serem removidos" estejam prontos (o que remove_ifdeve ser feito internamente). No entanto, se você tem um vectordos 5 elementos e você só .erase() 1 de cada vez, não há impacto no desempenho para o uso de iteradores vs remove_if. Se a lista for maior, você realmente deveria mudar para um local std::listonde há muitas remoções no meio da lista.
bobobobo
16

Idealmente, você não deve modificar o vetor durante a iteração sobre ele. Use o idioma apagar-remover. Se você fizer isso, provavelmente encontrará alguns problemas. Como em um vectoran eraseinvalida todos os iteradores começando com o elemento sendo apagado até o, end()você precisará se certificar de que seus iteradores permanecem válidos usando:

for (MyVector::iterator b = v.begin(); b != v.end();) { 
    if (foo) {
       b = v.erase( b ); // reseat iterator to a valid value post-erase
    else {
       ++b;
    }
}

Observe que você precisa do b != v.end()teste no estado em que se encontra. Se você tentar otimizá-lo da seguinte maneira:

for (MyVector::iterator b = v.begin(), e = v.end(); b != e;)

você irá se deparar com UB, pois seu eé invalidado após a primeira erasechamada.

diretamente
fonte
@ildjarn: Sim, é verdade! Isso foi um erro de digitação.
diretamente em
2
Este não é o idioma apagar-remover. Não há chamada para std::removee é O (N ^ 2), não O (N).
Potatoswatter
1
@Potatoswatter: Claro que não. Eu estava tentando apontar os problemas com a exclusão conforme você itera. Eu acho que minhas palavras não foram aprovadas?
dirkgently de
5

É um requisito estrito remover elementos durante esse loop? Caso contrário, você pode definir os ponteiros que deseja excluir como NULL e fazer outra passagem sobre o vetor para remover todos os ponteiros NULL.

std::vector<IInventory*> inv;
inv.push_back( new Foo() );
inv.push_back( new Bar() );

for ( IInventory* &index : inv )
{
    // do some stuff
    // ok I decided I need to remove this object from inv...?
    if (do_delete_index)
    {
        delete index;
        index = NULL;
    }
}
std::remove(inv.begin(), inv.end(), NULL);
Yexo
fonte
1

desculpe por necroposting e também desculpe se minha experiência em c ++ atrapalhar minha resposta, mas se você tentar iterar por cada item e fazer alterações possíveis (como apagar um índice), tente usar um backwords para loop.

for(int x=vector.getsize(); x>0; x--){

//do stuff
//erase index x

}

ao apagar o índice x, o próximo loop será para o item "na frente" da última iteração. eu realmente espero que isso ajude alguém

lilbigwill99
fonte
só não se esqueça ao usar x para acessar um determinado índice, faça x-1 lol
lilbigwill99
1

OK, estou atrasado, mas de qualquer maneira: Desculpe, não corrija o que li até agora - é possível, você só precisa de dois iteradores:

std::vector<IInventory*>::iterator current = inv.begin();
for (IInventory* index : inv)
{
    if(/* ... */)
    {
        delete index;
    }
    else
    {
        *current++ = index;
    }
}
inv.erase(current, inv.end());

Apenas modificar o valor para o qual um iterador aponta não invalida nenhum outro iterador, portanto, podemos fazer isso sem ter que nos preocupar. Na verdade, std::remove_if(implementação gcc pelo menos) faz algo muito semelhante (usando um loop clássico ...), apenas não exclui nada e não apaga.

Esteja ciente, no entanto, que isso não é seguro para thread (!) - no entanto, isso também se aplica a algumas das outras soluções acima ...

Aconcágua
fonte
Que diabos. Isso é um exagero.
Kesse de
@Kesse Sério? Este é o algoritmo mais eficiente que você pode obter com vetores (não importa se o loop baseado em intervalo ou o loop iterador clássico): Dessa forma, você move cada elemento no vetor no máximo uma vez e itera sobre o vetor inteiro exatamente uma vez. Quantas vezes você moveria os elementos subsequentes e iteraria no vetor se excluísse cada elemento correspondente viaerase (desde que exclua mais de um único elemento, é claro)?
Aconcágua
1

Vou mostrar com um exemplo, o exemplo abaixo remove elementos estranhos do vetor:

void test_del_vector(){
    std::vector<int> vecInt{0, 1, 2, 3, 4, 5};

    //method 1
    for(auto it = vecInt.begin();it != vecInt.end();){
        if(*it % 2){// remove all the odds
            it = vecInt.erase(it);
        } else{
            ++it;
        }
    }

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;

    // recreate vecInt, and use method 2
    vecInt = {0, 1, 2, 3, 4, 5};
    //method 2
    for(auto it=std::begin(vecInt);it!=std::end(vecInt);){
        if (*it % 2){
            it = vecInt.erase(it);
        }else{
            ++it;
        }
    }

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;

    // recreate vecInt, and use method 3
    vecInt = {0, 1, 2, 3, 4, 5};
    //method 3
    vecInt.erase(std::remove_if(vecInt.begin(), vecInt.end(),
                 [](const int a){return a % 2;}),
                 vecInt.end());

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;

}

saída aw abaixo:

024
024
024

Lembre-se de que o método eraseretornará o próximo iterador do iterador passado.

A partir daqui , podemos usar um método mais gerador:

template<class Container, class F>
void erase_where(Container& c, F&& f)
{
    c.erase(std::remove_if(c.begin(), c.end(),std::forward<F>(f)),
            c.end());
}

void test_del_vector(){
    std::vector<int> vecInt{0, 1, 2, 3, 4, 5};
    //method 4
    auto is_odd = [](int x){return x % 2;};
    erase_where(vecInt, is_odd);

    // output all the remaining elements
    for(auto const& it:vecInt)std::cout<<it;
    std::cout<<std::endl;    
}

Veja aqui para ver como usar std::remove_if . https://en.cppreference.com/w/cpp/algorithm/remove

Jayhello
fonte
1

Em oposição a este título de tópicos, eu usaria dois passes:

#include <algorithm>
#include <vector>

std::vector<IInventory*> inv;
inv.push_back(new Foo());
inv.push_back(new Bar());

std::vector<IInventory*> toDelete;

for (IInventory* index : inv)
{
    // Do some stuff
    if (deleteConditionTrue)
    {
        toDelete.push_back(index);
    }
}

for (IInventory* index : toDelete)
{
    inv.erase(std::remove(inv.begin(), inv.end(), index), inv.end());
}
Nikc
fonte
0

Uma solução muito mais elegante seria mudar para std::list(assumindo que você não precisa de acesso aleatório rápido).

list<Widget*> widgets ; // create and use this..

Você pode então excluir com .remove_ifum functor C ++ em uma linha:

widgets.remove_if( []( Widget*w ){ return w->isExpired() ; } ) ;

Então, aqui estou apenas escrevendo um functor que aceita um argumento (o Widget*). O valor de retorno é a condição na qual remover um Widget*da lista.

Acho essa sintaxe palatável. Eu não acho que eu nunca usaria remove_ifpara std :: vetores - há muito inv.begin()e inv.end()barulho lá você é provavelmente melhor fora de usar uma base-integer-índice de exclusão ou apenas uma planície antiga regular de exclusão (como mostrado à base de iterador abaixo). Mas você não deve realmente remover do meio de umstd::vector qualquer maneira, então listé aconselhável mudar para um neste caso de exclusão frequente no meio da lista.

Observe, entretanto, que não tive a chance de ligar deletepara os Widget*que foram removidos. Para fazer isso, ficaria assim:

widgets.remove_if( []( Widget*w ){
  bool exp = w->isExpired() ;
  if( exp )  delete w ;       // delete the widget if it was expired
  return exp ;                // remove from widgets list if it was expired
} ) ;

Você também pode usar um loop baseado em iterador regular como:

//                                                              NO INCREMENT v
for( list<Widget*>::iterator iter = widgets.begin() ; iter != widgets.end() ; )
{
  if( (*iter)->isExpired() )
  {
    delete( *iter ) ;
    iter = widgets.erase( iter ) ; // _advances_ iter, so this loop is not infinite
  }
  else
    ++iter ;
}

Se você não gosta do comprimento de for( list<Widget*>::iterator iter = widgets.begin() ; ..., você pode usar

for( auto iter = widgets.begin() ; ...
bobobobo
fonte
1
Não acho que você entenda como remove_ifum std::vectorfunciona realmente e como mantém a complexidade em O (N).
Ben Voigt
Isso não importa. Remover do meio de um std::vectorsempre fará deslizar cada elemento após o que você removeu, tornando uma std::listescolha muito melhor.
bobobobo
1
Não, não irá "deslizar cada elemento um para cima". remove_ifirá deslizar cada elemento para cima pelo número de espaços livres. No momento em que você contabiliza o uso do cache, remove_ifem um std::vectorprovavelmente supera a remoção de a std::list. E preserva o O(1)acesso aleatório.
Ben Voigt
1
Então você tem uma ótima resposta em busca de uma pergunta. Esta questão fala sobre como iterar a lista, que é O (N) para ambos os contêineres. E removendo elementos O (N), que também é O (N) para ambos os recipientes.
Ben Voigt
2
A pré-marcação não é necessária; é perfeitamente possível fazer isso em uma única passagem. Você só precisa manter o controle do "próximo elemento a ser inspecionado" e "próximo slot a ser preenchido". Pense nisso como construir uma cópia da lista, filtrada com base no predicado. Se o predicado retornar verdadeiro, pule o elemento, caso contrário, copie-o. Mas a cópia da lista é feita no local, e trocar / mover é usado em vez de copiar.
Ben Voigt
0

Acho que faria o seguinte ...

for (auto itr = inv.begin(); itr != inv.end();)
{
   // Do some stuff
   if (OK, I decided I need to remove this object from 'inv')
      itr = inv.erase(itr);
   else
      ++itr;
}
ajpieri
fonte
0

você não pode excluir o iterador durante a iteração do loop porque a contagem do iterador não corresponde e, após alguma iteração, você teria um iterador inválido.

Solução: 1) pegar a cópia do vetor original 2) iterar o iterador usando esta cópia 2) fazer algumas coisas e excluí-lo do vetor original.

std::vector<IInventory*> inv;
inv.push_back(new Foo());
inv.push_back(new Bar());

std::vector<IInventory*> copyinv = inv;
iteratorCout = 0;
for (IInventory* index : copyinv)
{
    // Do some stuff
    // OK, I decided I need to remove this object from 'inv'...
    inv.erase(inv.begin() + iteratorCout);
    iteratorCout++;
}  
Suraj Kumar
fonte
0

Apagar um elemento por um leva facilmente ao desempenho do N ^ 2. Melhor marcar os elementos que devem ser apagados e apagá-los imediatamente após o loop. Se eu posso presumir nullptr em um elemento inválido em seu vetor, então

std::vector<IInventory*> inv;
// ... push some elements to inv
for (IInventory*& index : inv)
{
    // Do some stuff
    // OK, I decided I need to remove this object from 'inv'...
    {
      delete index;
      index =nullptr;
    }
}
inv.erase( std::remove( begin( inv ), end( inv ), nullptr ), end( inv ) ); 

Deveria trabalhar.

Caso o seu "Faça alguma coisa" não esteja mudando elementos do vetor e apenas seja usado para tomar a decisão de remover ou manter o elemento, você pode convertê-lo para lambda (como foi sugerido em um post anterior de alguém) e usar

inv.erase( std::remove_if( begin( inv ), end( inv ), []( Inventory* i )
  {
    // DO some stuff
    return OK, I decided I need to remove this object from 'inv'...
  } ), end( inv ) );
Sergiy Kanilo
fonte