proper way to delete elements from a vector

[cayblood]

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<int> v;
v.push_back(1);
v.push_back(2);
v.push_back(3);
v.push_back(4);

for (vector<int>::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

 

[Grahamo]

have a look at this line........


iter = v.erase(iter);


can you see anything wrong with it?

G

 

[Clark S. Cox III]

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?

 

[peter koch]

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<int> v;
v.push_back(1);
v.push_back(2);
v.push_back(3);
v.push_back(4);

for (vector<int>::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(),predicate(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

 

[Valentin.Samko]

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.

 

[werasm]

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<int> v;
v.push_back(1);
v.push_back(2);
v.push_back(3);
v.push_back(4);

for (vector<int>::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 <iostream>
#include <vector>
#include <algorithm>
#include <functional>

template<class T>
struct print
{
void operator()( const T& t )
{
std::cout << t << std::endl;
}
};

int main(int argc, char* argv[])
{
typedef std::vector<int> 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<int>(), 2 ) ), v.end() );

std::for_each( v.begin(), v.end(), print<int>() );

return 0;
}

Regards,

Werner

 

[werasm]

have a look at this line........


iter = v.erase(iter);


can you see anything wrong with it?

Hmmm, me neither.

 

[werasm]

v.erase( std::remove_if( v.begin(), v.end(), std::bind2nd(

std::greater<int>(), 2 ) ), v.end() );

std::for_each( v.begin(), v.end(), print<int>() );

The for_each could be replaced by:

std::copy( v.begin(), v.end(), std:stream_iterator<int>(std::cout) );

The only difference is that this streams the data to std output
sequentially without cr/linefeed.

Regards,

W

 

[Grahamo]

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

 

[Neil Cerutti]

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<int> v;
v.push_back(1);
v.push_back(2);
v.push_back(3);
v.push_back(4);

for (vector<int>::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.

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<int>::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<int>::iterator i = remove_if(v.begin(), v.end(),
bind2nd(greater<int>(), 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<int>::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<int>(), 2))
, v.end());

 

[Valentin.Samko]

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]

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

 

[cayblood]

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, double> // 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 <class Arg, class Arg2, class Arg3, class Res> 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 <class Elem, class NodeName, class StateName> struct
node_has_state :
public ternary_function<Elem, NodeName, StateName, bool>
{
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(),predicate(2)),v.end());

 

[werasm]

On 2005-11-03, cayblood <[email protected]> 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<int>(), 2))
, v.end());

Yes, I could/should have explained it better. Thanks,

W

 

[Neil Cerutti]

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, double> // 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.