¿Eliminando el elemento del vector, mientras está en el bucle 'for' del rango C ++ 11?

97

Tengo un vector de IInventory *, y estoy recorriendo la lista usando el rango C ++ 11 para, para hacer cosas con cada uno.

Después de hacer algunas cosas con uno, es posible que desee eliminarlo de la lista y eliminar el objeto. Sé que puedo llamar deleteal puntero en cualquier momento para limpiarlo, pero ¿cuál es la forma correcta de eliminarlo del vector, mientras está en el forciclo de rango ? Y si lo elimino de la lista, ¿se invalidará mi bucle?

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
fuente
8
Si quiere ser elegante, puede usar std::remove_ifcon un predicado que "hace cosas" y luego devuelve verdadero si desea eliminar el elemento.
Benjamin Lindley
¿Hay alguna razón por la que no pueda simplemente agregar un contador de índice y luego usar algo como inv.erase (index)?
TomJ
4
@TomJ: Eso todavía arruinaría la iteración.
Ben Voigt
@TomJ y sería un asesino del rendimiento: en cada borrado, puedes mover todos los elementos después del borrado.
Ivan
1
@BenVoigt Recomendé cambiar a std::listabajo
bobobobo

Respuestas:

94

No, no puedes. Basado en rango fores para cuando necesita acceder a cada elemento de un contenedor una vez.

Debe usar el forciclo normal o uno de sus primos si necesita modificar el contenedor a medida que avanza, acceder a un elemento más de una vez o iterar de otra manera de manera no lineal a través del contenedor.

Por ejemplo:

auto i = std::begin(inv);

while (i != std::end(inv)) {
    // Do some stuff
    if (blah)
        i = inv.erase(i);
    else
        ++i;
}
Seth Carnegie
fuente
5
¿No se aplicaría el idioma de borrar-eliminar aquí?
Naveen
4
@Naveen Decidí no intentar hacer eso porque aparentemente él necesita iterar sobre cada elemento, hacer cálculos con él y luego posiblemente eliminarlo del contenedor. Erase-remove dice que simplemente borra los elementos para los que devuelve un predicado true, AFAIU, y parece mejor de esta manera no mezclar la lógica de iteración con el predicado.
Seth Carnegie
4
@SethCarnegie Erase-remove con una lambda para el predicado permite elegantemente eso (ya que esto ya es C ++ 11)
Potatoswatter
11
No me gusta esta solución, es O (N ^ 2) para la mayoría de los contenedores. remove_ifes mejor.
Ben Voigt
6
esta respuesta es correcta, erasedevuelve un nuevo iterador válido. Puede que no sea eficiente, pero está garantizado que funcionará.
sp2danny
56

Cada vez que se elimina un elemento del vector, debe asumir que los iteradores en o después del elemento borrado ya no son válidos, porque cada uno de los elementos que siguen al elemento borrado se mueve.

Un bucle for basado en rangos es simplemente azúcar sintáctico para un bucle "normal" que usa iteradores, por lo que se aplica lo anterior.

Dicho esto, simplemente podría:

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
fuente
5
" porque existe la posibilidad de que el vector reasigne el bloque de memoria en el que guarda sus elementos " No, vectornunca se reasignará debido a una llamada a erase. La razón por la que los iteradores se invalidan es porque cada uno de los elementos que siguen al elemento borrado se mueve.
ildjarn
2
Un valor predeterminado de captura de [&]sería apropiado, para permitirle "hacer algunas cosas" con variables locales.
Potatoswatter
2
Esto no se ve nada más sencillo que un bucle basado en iterador, además hay que recordar para rodear su remove_ifcon .erase, de lo contrario no pasa nada.
bobobobo
2
@bobobobo Si por "bucle basado en iteradores" te refieres a la respuesta de Seth Carnegie , esa es O (n ^ 2) en promedio. std::remove_ifEstá encendido).
Branko Dimitrijevic
Tiene un muy buen punto, intercambiando elementos al final de la lista y evitando mover elementos hasta después de que todos los elts "para ser eliminados" estén hechos (lo que remove_ifdebe hacerse internamente). Sin embargo, si usted tiene una vectorde 5 elementos y sólo .erase() 1 a la vez, no hay impacto en el rendimiento para el uso de iteradores vs remove_if. Si la lista es más grande, debería cambiar a std::listdonde hay mucha eliminación de la mitad de la lista.
bobobobo
16

Lo ideal es que no modifiques el vector mientras iteras sobre él. Utilice el modismo borrar-eliminar. Si lo hace, es probable que encuentre algunos problemas. Dado que en vectoran eraseinvalida todos los iteradores comenzando con el elemento que se borra hasta el end(), deberá asegurarse de que sus iteradores sigan siendo válidos utilizando:

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

Tenga en cuenta que necesita la b != v.end()prueba tal cual. Si intenta optimizarlo de la siguiente manera:

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

se encontrará con UB, ya que ese invalidará después de la primera erasellamada.

dirkgently
fuente
@ildjarn: ¡Sí, es verdad! Eso fue un error tipográfico.
dirkgently
2
Este no es el modismo de borrar-eliminar. No hay ninguna llamada a std::remove, y es O (N ^ 2) no O (N).
Potatoswatter
1
@Potatoswatter: Por supuesto que no. Estaba tratando de señalar los problemas con la eliminación mientras itera. ¿Supongo que mi redacción no pasó bien?
dirkgently
5

¿Es un requisito estricto eliminar elementos mientras se está en ese bucle? De lo contrario, podría establecer los punteros que desea eliminar en NULL y hacer otra pasada sobre el vector para eliminar todos los punteros 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
fuente
1

lo siento por necropostar y también lo siento si mi experiencia en c ++ se interpone en el camino de mi respuesta, pero si intenta iterar a través de cada elemento y hacer posibles cambios (como borrar un índice), intente usar un bucle de contraseñas.

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

//do stuff
//erase index x

}

al borrar el índice x, el siguiente ciclo será para el elemento "delante de" la última iteración. realmente espero que esto haya ayudado a alguien

lilbigwill99
fuente
simplemente no se olvide cuando use x para acceder a un índice determinado, haga x-1 lol
lilbigwill99
1

Está bien, llego tarde, pero de todos modos: lo siento, no corrijo lo que leí hasta ahora; es posible, solo necesitas dos iteradores:

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

La simple modificación del valor al que apunta un iterador no invalida ningún otro iterador, por lo que podemos hacer esto sin tener que preocuparnos. Realmente,std::remove_if (la implementación de gcc al menos) hace algo muy similar (usando un bucle clásico ...), simplemente no borra nada y no borra.

Sin embargo, tenga en cuenta que esto no es seguro para subprocesos (!); Sin embargo, esto también se aplica a algunas de las otras soluciones anteriores ...

Aconcagua
fuente
Que demonios. Esto es una exageración.
Kesse
@Kesse ¿De verdad? Este es el algoritmo más eficiente que puede obtener con vectores (no importa si es un bucle basado en rango o un bucle iterador clásico): de esta manera, mueve cada elemento en el vector como máximo una vez, e itera sobre el vector completo exactamente una vez. ¿Cuántas veces movería elementos subsiguientes e iteraría sobre el vector si eliminara cada elemento coincidente mediante erase(siempre que elimine más de un elemento, por supuesto)?
Aconcagua
1

Mostraré con un ejemplo, el siguiente ejemplo elimina elementos impares del vector:

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;

}

salida aw a continuación:

024
024
024

Tenga en cuenta que el método erasedevolverá el siguiente iterador del iterador pasado.

Desde aquí , podemos usar un método más generar:

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

Vea aquí para ver cómo se usa std::remove_if. https://en.cppreference.com/w/cpp/algorithm/remove

Jayhello
fuente
1

En oposición a este título de hilos, usaría dos pases:

#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
fuente
0

Una solución mucho más elegante sería cambiar a std::list(asumiendo que no necesita un acceso aleatorio rápido).

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

Luego puede eliminar con .remove_ify un functor de C ++ en una línea:

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

Así que aquí solo estoy escribiendo un functor que acepta un argumento (el Widget*). El valor de retorno es la condición en la que eliminar unWidget* de la lista.

Encuentro esta sintaxis agradable. No creo que lo use nunca remove_ifpara std :: vectors ; hay tanto inv.begin()y inv.end()ruido allí, probablemente sea mejor que use una eliminación basada en índices enteros o simplemente una eliminación basada en iteradores regulares antiguos (como se muestra abajo). Pero de std::vectortodos modos no debería eliminar de la mitad de un mucho, por lo que listse recomienda cambiar a una para este caso de eliminación frecuente de la mitad de la lista.

Sin embargo, tenga en cuenta que no tuve la oportunidad de llamar deletea los Widget*que se eliminaron. Para hacer eso, se vería así:

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

También puede usar un bucle regular basado en iteradores como este:

//                                                              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 ;
}

Si no le gusta la longitud de for( list<Widget*>::iterator iter = widgets.begin() ; ..., puede usar

for( auto iter = widgets.begin() ; ...
bobobobo
fuente
1
No creo que usted entiende cómo remove_ifen una std::vectorrealidad de trabajo, y cómo se mantiene la complejidad de O (N).
Ben Voigt
Eso no importa. Quitar desde el medio de a std::vectorsiempre va a deslizar cada elemento después del que quitó hacia arriba, haciendo una std::listelección mucho mejor.
bobobobo
1
No, no "deslizará cada elemento hacia arriba". remove_ifdeslizará cada elemento hacia arriba según el número de espacios liberados. Para cuando tenga en cuenta el uso de la caché, remove_ifes std::vectorprobable que supere la eliminación de un std::list. Y conserva O(1)el acceso aleatorio.
Ben Voigt
1
Entonces tienes una gran respuesta en busca de una pregunta. Esta pregunta habla de iterar la lista, que es O (N) para ambos contenedores. Y eliminando elementos O (N), que también es O (N) para ambos contenedores.
Ben Voigt
2
No es necesario marcar previamente; es perfectamente posible hacer esto en una sola pasada. Solo necesita realizar un seguimiento del "siguiente elemento que se inspeccionará" y "el siguiente espacio que se debe llenar". Piense en ello como crear una copia de la lista, filtrada según el predicado. Si el predicado devuelve verdadero, omita el elemento; de lo contrario, cópielo. Pero la copia de la lista se realiza en su lugar y se usa el intercambio / movimiento en lugar de copiar.
Ben Voigt
0

Creo que haría lo siguiente ...

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
fuente
0

no puede eliminar el iterador durante la iteración del bucle porque el recuento de iteradores no coincide y después de alguna iteración tendría un iterador no válido.

Solución: 1) tomar la copia del vector original 2) iterar el iterador usando esta copia 2) hacer algunas cosas y eliminarlo del vector 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
fuente
0

Borrar elementos uno por uno conduce fácilmente a un rendimiento N ^ 2. Es mejor marcar los elementos que deben borrarse y borrarlos inmediatamente después del bucle. Si puedo suponer nullptr en un elemento no válido en su vector, entonces

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

Deberia trabajar.

En caso de que su "Hacer algunas cosas" no cambie elementos del vector y solo se use para tomar la decisión de eliminar o mantener el elemento, puede convertirlo a lambda (como se sugirió en la publicación anterior de alguien) y 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
fuente