Menghapus item dari vektor, sementara di C ++ 11 range 'for' loop?

98

Saya memiliki vektor IInventory *, dan saya melakukan perulangan melalui daftar menggunakan C ++ 11 range for, untuk melakukan hal-hal dengan masing-masing.

Setelah melakukan beberapa hal dengannya, saya mungkin ingin menghapusnya dari daftar dan menghapus objeknya. Saya tahu saya dapat memanggil deletepenunjuk kapan saja untuk membersihkannya, tetapi apa cara yang tepat untuk menghapusnya dari vektor, saat berada dalam forlingkaran jangkauan ? Dan jika saya menghapusnya dari daftar akankah loop saya tidak valid?

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
sumber
8
Jika Anda ingin terlihat mewah, Anda dapat menggunakan std::remove_ifpredikat "melakukan sesuatu" dan mengembalikan nilai true jika Anda ingin elemen tersebut dihapus.
Benjamin Lindley
Apakah ada alasan mengapa Anda tidak bisa menambahkan penghitung indeks dan kemudian menggunakan sesuatu seperti inv.erase (index)?
TomJ
4
@TomJ: Itu masih akan mengacaukan iterasi.
Ben Voigt
@TomJ dan itu akan menjadi pembunuh kinerja - pada setiap penghapusan, Anda dapat memindahkan semua elemen setelah yang terhapus.
Ivan
1
@BenVoigt Saya merekomendasikan beralih ke di std::listbawah ini
bobobobo

Jawaban:

95

Tidak, tidak boleh. Berbasis rentang foradalah saat Anda perlu mengakses setiap elemen wadah satu kali.

Anda harus menggunakan forloop normal atau salah satu sepupunya jika Anda perlu mengubah penampung saat Anda melanjutkan, mengakses elemen lebih dari sekali, atau melakukan iterasi secara non-linier melalui penampung.

Sebagai contoh:

auto i = std::begin(inv);

while (i != std::end(inv)) {
    // Do some stuff
    if (blah)
        i = inv.erase(i);
    else
        ++i;
}
Seth Carnegie
sumber
5
Bukankah idiom hapus-hapus berlaku di sini?
Naveen
4
@Naveen Saya memutuskan untuk tidak mencoba melakukan itu karena tampaknya dia perlu mengulang setiap item, melakukan perhitungan dengannya, dan kemudian mungkin menghapusnya dari wadah. Hapus-hapus mengatakan bahwa Anda hanya menghapus elemen yang predikatnya dikembalikan true, AFAIU, dan tampaknya lebih baik cara ini tidak mencampur logika iterasi dengan predikat.
Seth Carnegie
4
@SethCarnegie Hapus-hapus dengan lambda untuk predikat dengan elegan memungkinkan itu (karena ini sudah C ++ 11)
Potatoswatter
11
Tidak suka solusi ini, ini adalah O (N ^ 2) untuk sebagian besar kontainer. remove_iflebih baik.
Ben Voigt
6
jawaban ini benar, erasemengembalikan iterator baru yang valid. mungkin tidak efisien, tetapi dijamin berhasil.
sp2danny
56

Setiap kali elemen dihapus dari vektor, Anda harus menganggap iterator pada atau setelah elemen terhapus tidak lagi valid, karena setiap elemen yang menggantikan elemen terhapus dipindahkan.

Perulangan-untuk berbasis rentang hanyalah gula sintaksis untuk perulangan "normal" yang menggunakan iterator, jadi hal di atas berlaku.

Karena itu, Anda cukup:

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
sumber
6
" karena ada kemungkinan bahwa vektor merelokasi blok memori di mana ia menyimpan elemen-elemennya " Tidak, a vectortidak akan pernah dialokasikan karena ada panggilan ke erase. Alasan iterator tidak valid adalah karena setiap elemen yang menggantikan elemen yang dihapus dipindahkan.
ildjarn
2
Sebuah capture-default [&]akan sesuai, untuk memungkinkannya "melakukan beberapa hal" dengan variabel lokal.
Potatoswatter
2
Ini tidak terlihat sederhana dari loop berbasis iterator, selain Anda harus ingat untuk mengelilingi Anda remove_ifdengan .erase, jika tidak ada yang terjadi.
bobobobo
3
@bobobo Jika dengan "loop berbasis iterator" yang Anda maksud adalah jawaban Seth Carnegie , yaitu rata-rata O (n ^ 2). std::remove_ifadalah O (n).
Branko Dimitrijevic
1
@bobobo Kecuali Anda benar - benar membutuhkan akses acak.
Branko Dimitrijevic
16

Idealnya, Anda tidak boleh memodifikasi vektor saat mengulanginya. Gunakan idiom hapus-hapus. Jika ya, Anda mungkin akan mengalami beberapa masalah. Karena dalam vectorsebuah erasemembatalkan semua iterator yang dimulai dengan elemen yang dihapus hingga end()Anda perlu memastikan bahwa iterator Anda tetap valid dengan menggunakan:

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

Perhatikan, bahwa Anda memerlukan b != v.end()tes apa adanya. Jika Anda mencoba mengoptimalkannya sebagai berikut:

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

Anda akan mengalami UB karena Anda etidak valid setelah erasepanggilan pertama .

dirkgently
sumber
@ildjarn: Ya, benar! Itu salah ketik.
dirkgently
2
Ini bukan idiom hapus-hapus. Tidak ada panggilan ke std::remove, dan itu O (N ^ 2) bukan O (N).
Potatoswatter
1
@Potatoswatter: Tentu saja tidak. Saya mencoba menunjukkan masalah penghapusan saat Anda mengulang. Saya kira kata-kata saya tidak berhasil?
dirkgently
5

Apakah merupakan persyaratan ketat untuk menghapus elemen saat berada dalam loop itu? Jika tidak, Anda dapat mengatur pointer yang ingin Anda hapus ke NULL dan membuat satu lagi melewati vektor untuk menghapus semua pointer 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
sumber
1

maaf untuk necroposting dan juga maaf jika keahlian c ++ saya menghalangi jawaban saya, tetapi jika Anda mencoba mengulang setiap item dan membuat kemungkinan perubahan (seperti menghapus indeks), coba gunakan backwords for loop.

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

//do stuff
//erase index x

}

saat menghapus indeks x, pengulangan berikutnya adalah untuk item "di depan" iterasi terakhir. Saya sangat berharap ini membantu seseorang

lilbigwill99
sumber
jangan lupa ketika menggunakan x untuk mengakses indeks tertentu, lakukan x-1 lol
lilbigwill99
1

OK, aku terlambat, tapi tetap: Maaf, tidak benar apa yang saya baca sejauh - itu adalah mungkin, Anda hanya perlu dua iterator:

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

Hanya memodifikasi nilai yang ditunjukkan oleh iterator tidak akan membatalkan iterator lain, jadi kita dapat melakukan ini tanpa harus khawatir. Sebenarnya, std::remove_if(setidaknya implementasi gcc) melakukan sesuatu yang sangat mirip (menggunakan loop klasik ...), hanya tidak menghapus apa pun dan tidak menghapus.

Namun, ketahuilah bahwa ini bukan thread safe (!) - namun, ini juga berlaku untuk beberapa solusi lain di atas ...

Aconcagua
sumber
Apa apaan. Ini berlebihan.
Kesse
@ Benarkah? Ini adalah algoritme paling efisien yang dapat Anda peroleh dengan vektor (tidak masalah apakah loop berbasis rentang atau loop iterator klasik): Dengan cara ini, Anda memindahkan setiap elemen dalam vektor paling banyak sekali, dan Anda mengulang seluruh vektor tepat satu kali. Berapa kali Anda akan memindahkan elemen berikutnya dan mengulangi vektor jika Anda menghapus setiap elemen yang cocok melalui erase(asalkan Anda menghapus lebih dari satu elemen, tentu saja)?
Aconcagua
1

Saya akan menunjukkan dengan contoh, contoh di bawah ini menghapus elemen ganjil dari vektor:

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;

}

keluaran aw di bawah ini:

024
024
024

Perlu diingat, metode ini eraseakan mengembalikan iterator berikutnya dari iterator yang diteruskan.

Dari sini , kita dapat menggunakan metode yang lebih banyak menghasilkan:

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

Lihat di sini untuk mengetahui cara menggunakan std::remove_if. https://en.cppreference.com/w/cpp/algorithm/remove

Jayhello
sumber
1

Bertentangan dengan judul utas ini, saya akan menggunakan dua umpan:

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

Solusi yang jauh lebih elegan adalah beralih ke std::list(dengan asumsi Anda tidak memerlukan akses acak cepat).

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

Anda kemudian dapat menghapus dengan .remove_ifdan fungsi C ++ dalam satu baris:

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

Jadi di sini saya hanya menulis sebuah functor yang menerima satu argumen (the Widget*). Nilai yang dikembalikan adalah kondisi untuk menghapus a Widget*dari daftar.

Menurut saya sintaks ini cocok. Saya tidak berpikir saya akan pernah menggunakan remove_ifuntuk std :: vektor - ada begitu banyak inv.begin()dan inv.end()kebisingan di sana Anda mungkin lebih baik menggunakan penghapusan berbasis indeks-integer atau hanya penghapusan berbasis iterator biasa biasa (seperti yang ditunjukkan di bawah). Tetapi Anda tidak boleh benar-benar menghapus dari tengah-tengah std::vector, jadi beralih ke a listuntuk kasus penghapusan daftar tengah yang sering ini disarankan.

Catatan namun saya tidak mendapatkan kesempatan untuk memanggil deletepada Widget*'s yang telah dihapus. Untuk melakukan itu, akan terlihat seperti ini:

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

Anda juga bisa menggunakan loop berbasis iterator biasa seperti:

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

Jika Anda tidak suka panjangnya for( list<Widget*>::iterator iter = widgets.begin() ; ..., Anda bisa menggunakan

for( auto iter = widgets.begin() ; ...
bobobobo
sumber
1
Saya tidak berpikir Anda mengerti bagaimana sebenarnya remove_ifsebuah std::vectorpekerjaan, dan bagaimana itu membuat kompleksitas menjadi O (N).
Ben Voigt
Itu tidak masalah. Menghapus dari tengah std::vectorakan selalu menggeser setiap elemen setelah Anda menghapus satu, membuat pilihan std::listyang jauh lebih baik.
bobobobo
1
Tidak, ini tidak akan "menggeser setiap elemen ke atas satu". remove_ifakan menggeser setiap elemen ke atas dengan jumlah spasi yang dibebaskan. Pada saat Anda menghitung penggunaan cache remove_ifpada std::vectorpenghapusan melebihi kemungkinan dari std::list. Dan mempertahankan O(1)akses acak.
Ben Voigt
1
Maka Anda memiliki jawaban yang bagus untuk mencari pertanyaan. Pertanyaan ini berbicara tentang pengulangan daftar, yaitu O (N) untuk kedua container. Dan menghapus elemen O (N), yang juga merupakan O (N) untuk kedua container.
Ben Voigt
2
Penandaan awal tidak diperlukan; sangat mungkin untuk melakukan ini dalam satu operan. Anda hanya perlu melacak "elemen berikutnya yang akan diperiksa" dan "slot berikutnya yang akan diisi". Anggap saja sebagai membangun salinan daftar, difilter berdasarkan predikat. Jika predikat mengembalikan nilai true, lewati elemen, jika tidak salin. Tetapi salinan daftar dibuat di tempatnya, dan pertukaran / pemindahan digunakan sebagai pengganti penyalinan.
Ben Voigt
0

Saya pikir saya akan melakukan yang berikut ...

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

Anda tidak dapat menghapus iterator selama iterasi loop karena jumlah iterator tidak cocok dan setelah beberapa iterasi Anda akan memiliki iterator yang tidak valid.

Solusi: 1) ambil salinan vektor asli 2) ulangi iterator menggunakan salinan ini 2) lakukan beberapa hal dan hapus dari vektor asli.

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

Menghapus elemen satu-per-satu dengan mudah mengarah ke kinerja N ^ 2. Lebih baik menandai elemen yang harus dihapus dan menghapusnya sekaligus setelah loop. Jika saya dapat menganggap nullptr dalam elemen tidak valid dalam vektor Anda, maka

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

harus bekerja.

Jika "Lakukan beberapa hal" Anda tidak mengubah elemen vektor dan hanya digunakan untuk membuat keputusan untuk menghapus atau menyimpan elemen, Anda dapat mengubahnya menjadi lambda (seperti yang disarankan di posting sebelumnya seseorang) dan menggunakan

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
sumber