Suppression d'un élément du vecteur, alors que dans la plage C ++ 11 boucle 'for'?

98

J'ai un vecteur de IInventory *, et je suis en train de parcourir la liste en utilisant la plage C ++ 11 pour faire des choses avec chacun.

Après avoir fait quelques trucs avec un, je veux peut-être le supprimer de la liste et supprimer l'objet. Je sais que je peux appeler deletele pointeur à tout moment pour le nettoyer, mais quelle est la bonne façon de le supprimer du vecteur, alors que dans la forboucle de plage ? Et si je le supprime de la liste, ma boucle sera-t-elle invalidée?

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
la source
8
Si vous voulez avoir de la fantaisie, vous pouvez l'utiliser std::remove_ifavec un prédicat qui "fait des trucs" et retourne ensuite true si vous voulez que l'élément soit supprimé.
Benjamin Lindley
Y a-t-il une raison pour laquelle vous ne pouvez pas simplement ajouter un compteur d'index puis utiliser quelque chose comme inv.erase (index)?
TomJ
4
@TomJ: Cela gâcherait encore l'itération.
Ben Voigt
@TomJ et ce serait un tueur de performances - à chaque effacement, vous pouvez déplacer tous les éléments après l'effacement.
Ivan
1
@BenVoigt J'ai recommandé de passer en std::listdessous
bobobobo

Réponses:

95

Non, tu ne peux pas. Basé sur la plage, forvous devez accéder une fois à chaque élément d'un conteneur.

Vous devez utiliser la forboucle normale ou l'un de ses cousins ​​si vous devez modifier le conteneur au fur et à mesure, accéder à un élément plus d'une fois ou effectuer une itération non linéaire dans le conteneur.

Par exemple:

auto i = std::begin(inv);

while (i != std::end(inv)) {
    // Do some stuff
    if (blah)
        i = inv.erase(i);
    else
        ++i;
}
Seth Carnegie
la source
5
L'idiome effacer-supprimer ne serait-il pas applicable ici?
Naveen
4
@Naveen J'ai décidé de ne pas essayer de faire ça parce qu'apparemment, il a besoin d'itérer sur chaque élément, de faire des calculs avec, puis éventuellement de le retirer du conteneur. Erase-remove indique que vous effacez simplement les éléments pour lesquels un prédicat retourne true, AFAIU, et il semble préférable de cette façon de ne pas mélanger la logique d'itération avec le prédicat.
Seth Carnegie
4
@SethCarnegie Erase-remove avec un lambda pour le prédicat permet élégamment cela (puisqu'il s'agit déjà de C ++ 11)
Potatoswatter
11
Je n'aime pas cette solution, c'est O (N ^ 2) pour la plupart des conteneurs. remove_ifest mieux.
Ben Voigt
6
cette réponse est correcte, eraserenvoie un nouvel itérateur valide. ce n'est peut-être pas efficace, mais il est garanti que cela fonctionne.
sp2danny
56

Chaque fois qu'un élément est supprimé du vecteur, vous devez supposer que les itérateurs à ou après l'élément effacé ne sont plus valides, car chacun des éléments succédant à l'élément effacé est déplacé.

Une boucle for basée sur une plage n'est que du sucre syntaxique pour une boucle "normale" utilisant des itérateurs, donc ce qui précède s'applique.

Cela étant dit, vous pouvez simplement:

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
la source
5
" car il y a une possibilité que le vecteur ait réalloué le bloc de mémoire dans lequel il garde ses éléments " Non, a vectorne sera jamais réalloué en raison d'un appel à erase. La raison pour laquelle les itérateurs sont invalidés est que chacun des éléments succédant à l'élément effacé est déplacé.
ildjarn
2
Une capture par défaut de [&]serait appropriée, pour lui permettre de "faire des choses" avec des variables locales.
Potatoswatter
2
Cela ne semble pas plus simple qu'une boucle basée iterator, en plus vous devez vous rappeler d'entourer votre remove_ifavec .erase, sinon rien ne se passe.
bobobobo
2
@bobobobo Si par "boucle basée sur un itérateur" vous entendez la réponse de Seth Carnegie , c'est O (n ^ 2) en moyenne. std::remove_ifest sur).
Branko Dimitrijevic
Vous avez un très bon point, en échangeant des éléments à la fin de la liste et en évitant de déplacer des éléments jusqu'à ce que tous les éléments "à supprimer" soient terminés (ce qui remove_ifdoit être fait en interne). Toutefois , si vous avez un vector5 éléments et vous ne .erase() 1 à un moment, il n'y a pas d' impact de la performance pour l' utilisation itérateurs vs remove_if. Si la liste est plus longue, vous devriez vraiment basculer std::listlà où il y a beaucoup de suppression au milieu de la liste.
bobobobo
16

Idéalement, vous ne devriez pas modifier le vecteur en l'itérant. Utilisez l'idiome effacer-supprimer. Si vous le faites, vous rencontrerez probablement quelques problèmes. Comme dans un vectoran eraseinvalide tous les itérateurs en commençant par l'élément en cours d'effacement jusqu'à la, end()vous devrez vous assurer que vos itérateurs restent valides en utilisant:

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

Notez que vous avez besoin du b != v.end()test tel quel. Si vous essayez de l'optimiser comme suit:

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

vous rencontrerez UB car votre eest invalidé après le premier eraseappel.

dirkgently
la source
@ildjarn: Oui, c'est vrai! C'était une faute de frappe.
dirkgently
2
Ce n'est pas l'idiome d'effacement-suppression. Il n'y a pas d'appel à std::remove, et c'est O (N ^ 2) pas O (N).
Potatoswatter
1
@Potatoswatter: Bien sûr que non. J'essayais de souligner les problèmes liés à la suppression lors de l'itération. Je suppose que mon libellé n'a pas réussi?
dirkgently
5

Est-ce une obligation stricte de supprimer des éléments dans cette boucle? Sinon, vous pouvez définir les pointeurs que vous souhaitez supprimer sur NULL et effectuer un autre passage sur le vecteur pour supprimer tous les pointeurs 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
la source
1

désolé pour le nécropostage et aussi désolé si mon expertise C ++ entrave ma réponse, mais si vous essayez d'itérer chaque élément et d'apporter des modifications possibles (comme l'effacement d'un index), essayez d'utiliser un backwords for loop.

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

//do stuff
//erase index x

}

lors de l'effacement de l'index x, la prochaine boucle sera pour l'élément "devant" la dernière itération. j'espère vraiment que cela a aidé quelqu'un

lilbigwill99
la source
n'oubliez pas lorsque vous utilisez x pour accéder à un certain index, faites x-1 lol
lilbigwill99
1

OK, je suis en retard, mais de toute façon: Désolé, pas correct ce que je lis à ce jour - il est possible, vous avez juste besoin de deux itérateurs:

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

Le simple fait de modifier la valeur vers laquelle pointe un itérateur n'invalide aucun autre itérateur, nous pouvons donc le faire sans nous inquiéter. En fait, std::remove_if(l'implémentation gcc au moins) fait quelque chose de très similaire (en utilisant une boucle classique ...), ne supprime rien et n'efface pas.

Sachez cependant que ce n'est pas thread-safe (!) - cependant, cela s'applique également à certaines des autres solutions ci-dessus ...

Aconcagua
la source
Que diable. C'est une exagération.
Kesse
@Kesse Vraiment? C'est l'algorithme le plus efficace que vous puissiez obtenir avec des vecteurs (peu importe s'il s'agit d'une boucle basée sur une plage ou d'une boucle d'itérateur classique): De cette façon, vous déplacez chaque élément du vecteur au plus une fois, et vous itérez sur tout le vecteur exactement une fois. Combien de fois déplaceriez-vous les éléments suivants et itéreriez-vous sur le vecteur si vous supprimiez chaque élément correspondant via erase(à condition que vous supprimiez plus d'un élément, bien sûr)?
Aconcagua
1

Je vais montrer avec l'exemple, l'exemple ci-dessous supprime les éléments impairs du vecteur:

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;

}

sortie aw ci-dessous:

024
024
024

Gardez à l'esprit que la méthode eraserenverra le prochain itérateur de l'itérateur passé.

À partir de , nous pouvons utiliser une méthode plus générer:

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

Voir ici pour voir comment l'utiliser std::remove_if. https://en.cppreference.com/w/cpp/algorithm/remove

Jayhello
la source
1

En opposition au titre de ce fil, j'utiliserais deux 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
la source
0

Une solution beaucoup plus élégante serait de passer à std::list(en supposant que vous n'ayez pas besoin d'un accès aléatoire rapide).

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

Vous pouvez ensuite supprimer avec .remove_ifet un foncteur C ++ en une seule ligne:

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

Alors ici, j'écris juste un foncteur qui accepte un argument (le Widget*). La valeur de retour est la condition sur laquelle supprimer a Widget*de la liste.

Je trouve cette syntaxe acceptable. Je ne pense pas que je pourrais jamais utiliser remove_ifpour std :: vecteurs - il y a tant inv.begin()et inv.end()bruit là , vous êtes probablement mieux à l'aide d' un indice basé entier suppression ou juste une plaine ancienne base iterator régulière de suppression (comme indiqué au dessous de). Mais vous ne devriez pas vraiment supprimer du milieu de std::vectorbeaucoup de toute façon, il listest donc conseillé de passer à un pour ce cas de suppression fréquente du milieu de la liste.

Notez cependant que je n'a pas eu l' occasion d'appeler deleteles Widget*« s qui ont été supprimés. Pour ce faire, cela ressemblerait à ceci:

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

Vous pouvez également utiliser une boucle basée sur un itérateur régulier comme ceci:

//                                                              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 vous n'aimez pas la longueur de for( list<Widget*>::iterator iter = widgets.begin() ; ..., vous pouvez utiliser

for( auto iter = widgets.begin() ; ...
bobobobo
la source
1
Je ne pense pas que vous compreniez comment remove_ifun std::vectortravail fonctionne réellement, et comment il maintient la complexité à O (N).
Ben Voigt
Cela n'a pas d'importance. Le retrait du milieu d'un std::vectorélément fera toujours glisser chaque élément après celui que vous avez retiré, ce qui en fait std::listun bien meilleur choix.
bobobobo
1
Non, il ne "fera pas glisser chaque élément vers le haut". remove_iffera glisser chaque élément vers le haut du nombre d'espaces libérés. Au moment où vous tenez compte de l'utilisation du cache, remove_ifsur une std::vectorsuppression probable surpasse d'un fichier std::list. Et préserve O(1)l'accès aléatoire.
Ben Voigt
1
Ensuite, vous avez une excellente réponse à la recherche d'une question. Cette question parle d'itérer la liste, qui est O (N) pour les deux conteneurs. Et en supprimant les éléments O (N), qui est également O (N) pour les deux conteneurs.
Ben Voigt
2
Le pré-marquage n'est pas nécessaire; il est parfaitement possible de le faire en un seul passage. Il vous suffit de garder une trace du "prochain élément à inspecter" et du "prochain emplacement à remplir". Considérez cela comme la construction d'une copie de la liste, filtrée en fonction du prédicat. Si le prédicat renvoie true, ignorez l'élément, sinon copiez-le. Mais la copie de liste est faite sur place, et l'échange / déplacement est utilisé au lieu de la copie.
Ben Voigt
0

Je pense que je ferais ce qui suit ...

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
la source
0

vous ne pouvez pas supprimer l'itérateur pendant l'itération de la boucle car le nombre d'itérateurs ne correspond pas et après une certaine itération, vous auriez un itérateur invalide.

Solution: 1) prendre la copie du vecteur original 2) itérer l'itérateur en utilisant cette copie 2) faire quelques trucs et le supprimer du vecteur 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
la source
0

Effacer l'élément un par un conduit facilement à des performances N ^ 2. Mieux vaut marquer les éléments à effacer et les effacer aussitôt après la boucle. Si je peux présumer nullptr dans un élément non valide dans votre vecteur, alors

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

devrait marcher.

Dans le cas où votre "Do some stuff" ne change pas les éléments du vecteur et n'est utilisé que pour prendre la décision de supprimer ou de conserver l'élément, vous pouvez le convertir en lambda (comme cela a été suggéré dans le message précédent de quelqu'un) et utiliser

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
la source