You have three bugs here.
for (vector<File>::const_iterator it = this->files.begin(); it != this->files.end(); ++it) {
if (...) erase(it); // break after, no need of ++it in else branch
}
The first bug is that you incorrectly cut-and-pasted your code into StackOverflow. What you meant to paste was
for (vector<File>::const_iterator it = this->files.begin(); it != this->files.end(); ++it) {
if (...) this->files.erase(it); // break after, no need of ++it in else branch
}
The second bug is what the compiler is warning you about: there's no way to modify a collection through a const_iterator
. (EDIT: Okay, apparently C++11 added such a way, but libstdc++ didn't support it immediately.) That's what the const_
part means! If you want to modify the collection, use a plain old iterator
:
for (vector<File>::iterator it = this->files.begin(); it != this->files.end(); ++it) {
if (...) this->files.erase(it); // LOOK OUT, THERE'S STILL A BUG
}
The third bug is that once you call std::vector::erase
on a collection, all iterators (and const_iterator
s) into that collection become unusable. The Standard terminology for this is that erase
invalidates iterators. (The reason for this is that std::vector
behaves basically like a big heap-allocated buffer, and calling resize
on the vector is allowed to do the equivalent of realloc
(1) on the buffer, and calling erase
is allowed to call resize
(because if you erase
half the elements in the vector, you're probably expecting the heap-allocation to shrink accordingly).)
So, what you're trying to do won't work using that naive for-loop approach. What you need to do is use a standard algorithm, namely remove_if
:
#include <algorithm>
auto predicate = [](const File& f) { return f.ShouldBeErasedOrWhatever(); }
auto newEndIterator = std::remove_if(this->files.begin(), this->files.end(), predicate);
this->files.erase(newEndIterator, this->files.end()); // erase everything after "newEndIterator"
Replace f.ShouldBeErasedOrWhatever()
with whatever your "...
" is in the original code. Now you have valid, idiomatic C++11 that does the right thing — with no bugs!
(1) – Note on "the equivalent of realloc
": Of course it's not really realloc
; it's really a type-safe process that calls move-constructors and destructors as needed. vector
knows it's not generally safe to memcpy
arbitrary objects in C++.