454,435 Members | 1,499 Online Need help? Post your question and get tips & solutions from a community of 454,435 IT Pros & Developers. It's quick & easy.

# proper way to delete elements from a vector

 P: n/a I want to iterate through a vector and erase elements that meet a certain criteria. I know there is an algorithmic way of doing this, but first I would like to know how to do it with normal iteration. I'm am trying to do something like: vector v; v.push_back(1); v.push_back(2); v.push_back(3); v.push_back(4); for (vector::iterator iter = v.begin(); iter != v.end(); ++iter) { if (*iter > 2) iter = v.erase(iter); } This is giving me an error saying: error: void value not ignored as it ought to be. I'm sure what I'm doing is wrong, but I can't find a good example of the right way to do this. Thanks, Carl Nov 3 '05 #1
14 Replies

 P: n/a have a look at this line........ iter = v.erase(iter); can you see anything wrong with it? G Nov 3 '05 #2

 P: n/a On 2005-11-03 06:09:32 -0500, "Gr*****@nospam.com" said: have a look at this line........ iter = v.erase(iter); can you see anything wrong with it? No, what do you think is wrong with it? -- Clark S. Cox, III cl*******@gmail.com Nov 3 '05 #3

 P: n/a cayblood skrev: I want to iterate through a vector and erase elements that meet a certain criteria. I know there is an algorithmic way of doing this, but first I would like to know how to do it with normal iteration. I'm am trying to do something like: vector v; v.push_back(1); v.push_back(2); v.push_back(3); v.push_back(4); for (vector::iterator iter = v.begin(); iter != v.end(); ++iter) { if (*iter > 2) iter = v.erase(iter); } This is giving me an error saying: error: void value not ignored as it ought to be. I'm sure what I'm doing is wrong, but I can't find a good example of the right way to do this Use the standard library for this: std::erase(std::remove_if(v.begin(),v.end(),pred), v.end()); where pred is the predicate (designating the elements you want to erase). If you use boost::lambda, pred is as simple as _1 > 2. Otherwise you'll have to use bind and std::greater (if i remember correctly) or write your own predicate: struct predicate { predicate(int val): val_(val) {} bool operator<(int i) { return i > val_; } private: int val_; }; and call it like this: std::erase(std::remove_if(v.begin(),v.end(),predic ate(2)),v.end()); In real code, predicate should be renamed to e.g. less_than. Perhaps there is already some std support for this. /Peter Thanks, Carl Nov 3 '05 #4

 P: n/a > This is giving me an error saying: error: void value not ignored as it ought to be. I'm sure what I'm doing is wrong, but I can't find a good example of the right way to do this. Which compiler are you using? I do not see any technical errors in your code. -- Valentin Samko - http://www.valentinsamko.com Nov 3 '05 #5

 P: n/a cayblood wrote: I want to iterate through a vector and erase elements that meet a certain criteria. I know there is an algorithmic way of doing this, but first I would like to know how to do it with normal iteration. I'm am trying to do something like: vector v; v.push_back(1); v.push_back(2); v.push_back(3); v.push_back(4); for (vector::iterator iter = v.begin(); iter != v.end(); ++iter) { if (*iter > 2) iter = v.erase(iter); } You forgot to qualify vector with std. vector exists in the standard namespace. The following code was compiled with VC7.1. It is almost exactly yours, apart from me qualifying vector with std. Note also the algorthmic way. Your way was commented out, but does the same thing - BTW, you had an error in your code. When an item is erased, the following item remained because you should conditionally increment... Here goes: // CompileTest.cpp : Defines the entry point for the console application. // #include #include #include #include template struct print { void operator()( const T& t ) { std::cout << t << std::endl; } }; int main(int argc, char* argv[]) { typedef std::vector ivect; ivect v; v.push_back(1); v.push_back(2); v.push_back(3); v.push_back(4); //for( ivect::iterator iter = v.begin(); iter != v.end(); ) //{ // if (*iter > 2) iter = v.erase(iter); // else ++iter; //} v.erase( std::remove_if( v.begin(), v.end(), std::bind2nd( std::greater(), 2 ) ), v.end() ); std::for_each( v.begin(), v.end(), print() ); return 0; } Regards, Werner Nov 3 '05 #6

 P: n/a Gr*****@nospam.com wrote: have a look at this line........ iter = v.erase(iter); can you see anything wrong with it? Hmmm, me neither. G Nov 3 '05 #7

 P: n/a v.erase( std::remove_if( v.begin(), v.end(), std::bind2nd( std::greater(), 2 ) ), v.end() ); std::for_each( v.begin(), v.end(), print() ); The for_each could be replaced by: std::copy( v.begin(), v.end(), std::ostream_iterator(std::cout) ); The only difference is that this streams the data to std output sequentially without cr/linefeed. Regards, W Nov 3 '05 #8

 P: n/a apologies.... should have checked the return value from erase. My mistake. It would have been a runtime error not acompile error in any case G Nov 3 '05 #9

 P: n/a On 2005-11-03, cayblood wrote: I want to iterate through a vector and erase elements that meet a certain criteria. I know there is an algorithmic way of doing this, but first I would like to know how to do it with normal iteration. I'm am trying to do something like: vector v; v.push_back(1); v.push_back(2); v.push_back(3); v.push_back(4); for (vector::iterator iter = v.begin(); iter != v.end(); ++iter) { if (*iter > 2) iter = v.erase(iter); } vector::erase returns an interator to the next element in the vector, so in that case you mustn't increment iter. This is giving me an error saying: error: void value not ignored as it ought to be. The code you posted doesn't seem to have that error. Post actual code whenever possible. If it's an except, make it a complete and compilable excerpt. Here's one example of using erase in a loop: for (vector::iterator i = v.begin(); i != v.end(); /* */) { if (*i > 2) { i = v.erase(i); } else { ++i; } } Using the standard functions remove_if and erase (as shown in another post) will be better in most cases. Note that remove_if does not actually erase elements. It swaps elements that must be retained to the front of the container and returns an iterator to the end of the swapped elements. So it's usually combined with erase. Assuming v is (1, 2, 3, 4), as above: vector::iterator i = remove_if(v.begin(), v.end(), bind2nd(greater(), 2)); If the standard functions objects are a mystery, you may define a function instead. bool greater_than_2(int i) { return i > 2; } vector::iterator i = remove_if(v.begin(), v.end(), greater_than_2); After either of those remove_if calls, v contains (1, 2, 3, 4). Since the retained alements were already in front, nothing happened. However, i now points to 3, and you can erase from there to v.end() to get your final answer. v.erase(i, v.end()); As shown in another poset, a popular idiom is to combine the calls into one line, eliminating the temporary iterator. v.erase(remove_if(v.begin(), v.end(), bind2nd(greater(), 2)) , v.end()); -- Neil Cerutti Nov 3 '05 #10

 P: n/a By "technical", I meant "compile time" errors here. You have a logical problem, where you move to the next element calling iter = erase(iter) (iter may be .end() at this point), and then you increment your iterator, which leads to UB if iter was already pointing to the end. Even if this doesn't happen, you will skip elements by effectively incrementing your iterator twice (when you erase). -- Valentin Samko - http://www.valentinsamko.com Nov 3 '05 #11

 P: n/a By "technical", I meant "compile time" errors here. You have a logical problem, where you move to the next element calling iter = erase(iter) (iter may be .end() at this point), and then you increment your iterator, which leads to UB if iter was already pointing to the end. Even if this doesn't happen, you will skip elements by effectively incrementing your iterator twice (when you erase). -- Valentin Samko - http://www.valentinsamko.com Nov 3 '05 #12

 P: n/a I need a slightly more complicated predicate and I'm having a hard time creating it. I have a map that looks like this map // Event is a user-defined class I want to remove all elements of this map where event.node_has_state(name, state). In other words, I need to call a method on the first part of the pair and pass it two arguments. I tried creating the following: // predicate classes and functions for three argument predicates template struct ternary_function { typedef Arg argument_type; typedef Arg2 argument_type; typedef Arg3 argument_type; typedef Res result_type; }; // predicate for testing whether a node is in a certain state template struct node_has_state : public ternary_function { bool operator()(const Elem& e, const NodeName& n, const StateName& s) const { return e.node_has_state(n, s); } }; But I think that these work with vectors but not maps. Any suggestions? Thanks, Carl Original msg: --------------------------------------- Peter Koch wrote: Use the standard library for this: std::erase(std::remove_if(v.begin(),v.end(),pred), v.end()); where pred is the predicate (designating the elements you want to erase). If you use boost::lambda, pred is as simple as _1 > 2. Otherwise you'll have to use bind and std::greater (if i remember correctly) or write your own predicate: struct predicate { predicate(int val): val_(val) {} bool operator<(int i) { return i > val_; } private: int val_; }; and call it like this: std::erase(std::remove_if(v.begin(),v.end(),predic ate(2)),v.end()); Nov 4 '05 #13

 P: n/a Neil Cerutti wrote: On 2005-11-03, cayblood wrote: As shown in another poset, a popular idiom is to combine the calls into one line, eliminating the temporary iterator. v.erase(remove_if(v.begin(), v.end(), bind2nd(greater(), 2)) , v.end()); Yes, I could/should have explained it better. Thanks, W -- Neil Cerutti Nov 4 '05 #14

 P: n/a On 2005-11-04, cayblood wrote: I need a slightly more complicated predicate and I'm having a hard time creating it. I have a map that looks like this map // Event is a user-defined class I want to remove all elements of this map where event.node_has_state(name, state). In other words, I need to call a method on the first part of the pair and pass it two arguments. You cannot use remove_if on a map, since the key elements are stored as const objects. Use the map member functions find and erase, instead. -- Neil Cerutti Nov 4 '05 #15

### This discussion thread is closed

Replies have been disabled for this discussion. 