Usuwanie elementu z wektora, podczas gdy w C ++ 11 pętla „for”?

98

Mam wektor IInventory * i przeglądam listę przy użyciu zakresu C ++ 11 dla, aby robić rzeczy z każdym z nich.

Po zrobieniu kilku rzeczy z jednym, mogę chcieć usunąć go z listy i usunąć obiekt. Wiem, że mogę deletew każdej chwili wywołać wskaźnik, aby go wyczyścić, ale jaki jest właściwy sposób usunięcia go z wektora, gdy jestem w forpętli zakresu ? A jeśli usunę go z listy, czy moja pętla zostanie unieważniona?

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
źródło
8
Jeśli chcesz uzyskać wymyślny wygląd, możesz użyć std::remove_ifpredykatu, który „robi rzeczy”, a następnie zwraca prawdę, jeśli chcesz usunąć element.
Benjamin Lindley
Czy jest powód, dla którego nie możesz po prostu dodać licznika indeksu, a następnie użyć czegoś takiego jak inv.erase (index)?
TomJ
4
@TomJ: To i tak schrzaniłoby iterację.
Ben Voigt
@TomJ i byłby to zabójca wydajności - przy każdym wymazaniu możesz przesuwać wszystkie elementy po usuniętym.
Ivan
1
@BenVoigt Poleciłem przejście na std::listniższy
bobobobo

Odpowiedzi:

95

Nie, nie możesz. Oparty na zakresie forjest używany, gdy musisz raz uzyskać dostęp do każdego elementu kontenera.

Powinieneś użyć normalnej forpętli lub jednego z jej kuzynów, jeśli musisz zmodyfikować kontener w trakcie, uzyskać dostęp do elementu więcej niż jeden raz lub w inny sposób iterować w nieliniowy sposób przez kontener.

Na przykład:

auto i = std::begin(inv);

while (i != std::end(inv)) {
    // Do some stuff
    if (blah)
        i = inv.erase(i);
    else
        ++i;
}
Seth Carnegie
źródło
5
Czy nie miałby zastosowania tutaj idiom erase-remove?
Naveen
4
@Naveen Postanowiłem nie próbować tego robić, ponieważ najwyraźniej musi iterować każdy element, wykonać z nim obliczenia, a następnie ewentualnie usunąć go z kontenera. Erase-remove mówi, że po prostu truekasujesz elementy, dla których zwraca się predykat , AFAIU, i wydaje się, że lepiej w ten sposób nie mieszać logiki iteracji z predykatem.
Seth Carnegie
4
@SethCarnegie Erase-remove z lambda dla predykatu elegancko na to pozwala (ponieważ jest to już C ++ 11)
Potatoswatter
11
Nie podoba mi się to rozwiązanie, dla większości kontenerów jest to O (N ^ 2). remove_ifjest lepiej.
Ben Voigt
6
ta odpowiedź jest poprawna, erasezwraca nowy prawidłowy iterator. może nie być wydajne, ale na pewno zadziała.
sp2danny
56

Za każdym razem, gdy element jest usuwany z wektora, należy założyć, że iteratory na lub po usuniętym elemencie nie są już prawidłowe, ponieważ każdy z elementów występujących po usuniętym elemencie jest przesuwany.

Oparta na zakresie pętla for jest po prostu cukrem syntaktycznym dla „normalnej” pętli używającej iteratorów, więc powyższe ma zastosowanie.

Biorąc to pod uwagę, możesz po prostu:

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
źródło
6
ponieważ istnieje możliwość, że wektor ponownie przydzielił blok pamięci, w którym przechowuje swoje elementy ” Nie, vectornigdy nie zostanie ponownie przydzielony z powodu wywołania erase. Powodem unieważnienia iteratorów jest to, że każdy z elementów następujących po usuniętym elemencie jest przenoszony.
ildjarn
2
Domyślne przechwytywanie [&]byłoby odpowiednie, aby umożliwić mu „zrobienie czegoś” ze zmiennymi lokalnymi.
Potatoswatter
2
To nie wygląda prostsze pętli iteracyjnej oparte dodatkowo trzeba pamiętać, aby otoczyć remove_ifsię .erase, w przeciwnym razie nic się nie dzieje.
bobobobo
3
@bobobobo Jeśli przez „pętlę opartą na iteratorze” masz na myśli odpowiedź Setha Carnegiego , to jest to średnio O (n ^ 2). std::remove_ifjest O (n).
Branko Dimitrijevic
1
@bobobobo Chyba że faktycznie potrzebujesz dostępu losowego.
Branko Dimitrijevic
16

Idealnie nie powinieneś modyfikować wektora podczas iteracji po nim. Użyj idiomu erase-remove. Jeśli to zrobisz, prawdopodobnie napotkasz kilka problemów. Ponieważ w vectoran eraseunieważnia wszystkie iteratory począwszy od elementu są usuwane upto end()trzeba upewnić się, że Iteratory zachowują ważność za pomocą:

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

Zauważ, że potrzebujesz b != v.end()testu tak jak jest. Jeśli spróbujesz go zoptymalizować w następujący sposób:

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

natkniesz się na UB, ponieważ Twój ejest unieważniony po pierwszym erasewywołaniu.

dirkgently
źródło
@ildjarn: Tak, prawda! To była literówka.
dirkgently
2
To nie jest idiom z wymazaniem. Nie ma wywołania std::removei jest to O (N ^ 2), a nie O (N).
Potatoswatter
1
@Potatoswatter: Oczywiście, że nie. Próbowałem zwrócić uwagę na problemy z usuwaniem podczas iteracji. Myślę, że moje sformułowanie nie przeszło apelu?
dirkgently
5

Czy usuwanie elementów w tej pętli jest ścisłym wymogiem? W przeciwnym razie możesz ustawić wskaźniki, które chcesz usunąć, na NULL i wykonać kolejny przejazd nad wektorem, aby usunąć wszystkie wskaźniki 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
źródło
1

przepraszam za nekropostowanie i również przepraszam, jeśli moja wiedza na temat c ++ przeszkadza mi w udzieleniu odpowiedzi, ale jeśli próbujesz iterować przez każdy element i wprowadzać możliwe zmiany (takie jak wymazanie indeksu), spróbuj użyć backwords for loop.

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

//do stuff
//erase index x

}

podczas kasowania indeksu x następna pętla będzie dotyczyła elementu znajdującego się „przed” ostatnią iteracją. Naprawdę mam nadzieję, że to komuś pomogło

lilbigwill99
źródło
po prostu nie zapomnij, kiedy używasz x do dostępu do określonego indeksu, zrób x-1 lol
lilbigwill99
1

OK, spóźniłem się, ale i tak: przepraszam, nie poprawiam tego, co przeczytałem do tej pory - jest możliwe, potrzebujesz tylko dwóch iteratorów:

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

Samo zmodyfikowanie wartości, na którą wskazuje iterator, nie unieważni żadnego innego iteratora, więc możemy to zrobić bez obaw. Tak właściwie,std::remove_if (przynajmniej implementacja gcc) robi coś bardzo podobnego (używając klasycznej pętli ...), po prostu niczego nie usuwa i nie kasuje.

Należy jednak pamiętać, że nie jest to bezpieczne wątkowo (!) - dotyczy to jednak również niektórych innych rozwiązań powyżej ...

Aconcagua
źródło
Co za cholera. To przesada.
Kesse
@Kesse Naprawdę? Jest to najskuteczniejszy algorytm, jaki można uzyskać za pomocą wektorów (nie ma znaczenia, czy pętla oparta na zakresie, czy klasyczna pętla iteratora): w ten sposób każdy element w wektorze przesuwa się najwyżej raz i dokładnie raz iteruje się po całym wektorze. Ile razy przesuwałbyś kolejne elementy i iterował po wektorze, gdybyś usunął każdy pasujący element przez erase(oczywiście pod warunkiem, że usuniesz więcej niż jeden element)?
Aconcagua,
1

Pokażę na przykładzie, poniższy przykład usuwa nieparzyste elementy z wektora:

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;

}

wyjście aw poniżej:

024
024
024

Należy pamiętać, że metoda erasezwróci następny iterator przekazanego iteratora.

Od tutaj możemy zastosować metodę bardziej generowania:

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

Zobacz tutaj, aby zobaczyć, jak używać std::remove_if. https://en.cppreference.com/w/cpp/algorithm/remove

Jayhello
źródło
1

W przeciwieństwie do tego tytułu wątków użyłbym dwóch przejść:

#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
źródło
0

O wiele bardziej eleganckim rozwiązaniem byłoby przejście na std::list(zakładając, że nie potrzebujesz szybkiego losowego dostępu).

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

Następnie możesz usunąć za pomocą .remove_ifi funktora C ++ w jednej linii:

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

Więc tutaj piszę po prostu funktor, który akceptuje jeden argument (the Widget*). Wartość zwracana to warunek, w którym należy usunąć Widget*z listy.

Uważam, że ta składnia jest do przyjęcia. Nie sądzę, abym kiedykolwiek używał remove_ifdla std :: vectors - jest tam tak dużo inv.begin()i inv.end()szumu, że prawdopodobnie lepiej będzie użyć usuwania opartego na indeksach całkowitych lub po prostu zwykłego starego regularnego usuwania opartego na iteratorze (jak pokazano poniżej). Ale tak naprawdę nie powinieneś usuwać z połowy std::vectorbardzo dużo, więc listzaleca się przełączenie na a w tym przypadku częstego usuwania środka listy.

Zauważ jednak, że nie miałem szansy zadzwonić deletena te Widget*, które zostały usunięte. Aby to zrobić, wyglądałoby to tak:

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

Możesz także użyć zwykłej pętli opartej na iteratorze, na przykład:

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

Jeśli nie podoba Ci się długość for( list<Widget*>::iterator iter = widgets.begin() ; ..., możesz użyć

for( auto iter = widgets.begin() ; ...
bobobobo
źródło
1
Nie sądzę, że rozumiesz, jak remove_ifw std::vectorrzeczywistości działa i jak utrzymuje złożoność na poziomie O (N).
Ben Voigt
To nie ma znaczenia. Usunięcie ze środka a std::vectorzawsze przesunie każdy element po tym, który usunąłeś, co jest std::listo wiele lepszym wyborem.
bobobobo
1
Nie, to nie „przesunie każdego elementu o jeden”. remove_ifprzesunie każdy element w górę o liczbę zwolnionych przestrzeni. Do czasu, kiedy stanowią cache, remove_ifna std::vectorktóre prawdopodobnie usunięcie przewyższa od A std::list. I zachowuje O(1)swobodny dostęp.
Ben Voigt
1
Wtedy masz świetną odpowiedź w poszukiwaniu pytania. To pytanie dotyczy iteracji listy, która jest O (N) dla obu kontenerów. I usunięcie elementów O (N), co jest również O (N) dla obu pojemników.
Ben Voigt
2
Znakowanie wstępne nie jest wymagane; można to zrobić za jednym razem. Musisz tylko śledzić „następny element do sprawdzenia” i „następny boks do wypełnienia”. Potraktuj to jako tworzenie kopii listy przefiltrowanej na podstawie predykatu. Jeśli predykat zwraca prawdę, pomiń element, w przeciwnym razie skopiuj go. Ale kopia listy jest tworzona na miejscu, a zamiast kopiowania używana jest zamiana / przenoszenie.
Ben Voigt
0

Myślę, że zrobiłbym co następuje ...

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
źródło
0

nie możesz usunąć iteratora podczas iteracji pętli, ponieważ liczba iteratorów jest niedopasowana i po pewnej iteracji miałbyś nieprawidłowy iterator.

Rozwiązanie: 1) weź kopię oryginalnego wektora 2) przeprowadź iterację iteratora używając tej kopii 2) zrób coś i usuń go z oryginalnego wektora.

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
źródło
0

Wymazywanie elementu jeden po drugim łatwo prowadzi do wydajności N ^ 2. Lepiej jest zaznaczyć elementy, które powinny zostać usunięte i usunąć je od razu po wykonaniu pętli. Jeśli mogę założyć, że nullptr jest nieprawidłowym elementem w twoim wektorze, to

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

powinno działać.

W przypadku, gdy twoje "Zrób coś" nie zmienia elementów wektora i służy tylko do podjęcia decyzji o usunięciu lub zachowaniu elementu, możesz przekonwertować go na lambdę (jak zasugerowano we wcześniejszym poście) i użyć

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
źródło