Question

I can't exmplain this behaviour:

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
}

where File is my own class (std not included) and this->files is vector of Files

when I compile the code I get (see line 2)

Path.cpp: In member function ‘void Path::rmFile(File&)’:
Path.cpp:190:24: error: no matching function for call to ‘std::vector<File>::erase(std::vector<File>::const_iterator&)’
Path.cpp:190:24: note: candidates are:
In file included from /usr/include/c++/4.7/vector:70:0,
             from Path.h:5,
             from Path.cpp:1:
/usr/include/c++/4.7/bits/vector.tcc:135:5: note: std::vector<_Tp, _Alloc>::iterator     std::vector<_Tp, _Alloc>::erase(std::vector<_Tp, _Alloc>::iterator) [with _Tp = File; _Alloc =     std::allocator<File>; std::vector<_Tp, _Alloc>::iterator = __gnu_cxx::__normal_iterator<File*,     std::vector<File> >; typename std::_Vector_base<_Tp, _Alloc>::pointer = File*]
/usr/include/c++/4.7/bits/vector.tcc:135:5: note:   no known conversion for argument 1     from ‘std::vector<File>::const_iterator {aka __gnu_cxx::__normal_iterator<const File*,     std::vector<File> >}’ to ‘std::vector<File>::iterator {aka __gnu_cxx::__normal_iterator<File*,     std::vector<File> >}’
/usr/include/c++/4.7/bits/vector.tcc:147:5: note: std::vector<_Tp, _Alloc>::iterator     std::vector<_Tp, _Alloc>::erase(std::vector<_Tp, _Alloc>::iterator, std::vector<_Tp,     _Alloc>::iterator) [with _Tp = File; _Alloc = std::allocator<File>; std::vector<_Tp,     _Alloc>::iterator = __gnu_cxx::__normal_iterator<File*, std::vector<File> >; typename     std::_Vector_base<_Tp, _Alloc>::pointer = File*]
/usr/include/c++/4.7/bits/vector.tcc:147:5: note:   candidate expects 2 arguments, 1 provided
make: *** [Path.o] Error 1

even the doc says it's ok, but error no matching function for call to std::vector::erase(std::vector::const_iterator&) is really wierd.

I really need to be able to delete vector item by iterator. Can anybody help me please? Thaks in advance.

Was it helpful?

Solution

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_iterators) 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++.

OTHER TIPS

Assuming your example code is incorrect and it really is files.erase(it), then const_iterator version was only added in C++11 which it looks like you don't have since you're not using auto.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top