proper way to delete elements from a vector

Discussion in 'C++' started by cayblood, Nov 3, 2005.

  1. cayblood

    cayblood Guest

    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
     
    cayblood, Nov 3, 2005
    #1
    1. Advertising

  2. cayblood

    Guest

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


    iter = v.erase(iter);


    can you see anything wrong with it?

    G
     
    , Nov 3, 2005
    #2
    1. Advertising

  3. On 2005-11-03 06:09:32 -0500, ""
    <> 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
     
    Clark S. Cox III, Nov 3, 2005
    #3
  4. cayblood

    peter koch Guest

    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
    >
    > Thanks,
    > Carl
     
    peter koch, Nov 3, 2005
    #4
  5. > 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
     
    Valentin.Samko, Nov 3, 2005
    #5
  6. cayblood

    werasm Guest

    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<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, Nov 3, 2005
    #6
  7. cayblood

    werasm Guest

    wrote:
    > have a look at this line........
    >
    >
    > iter = v.erase(iter);
    >
    >
    > can you see anything wrong with it?


    Hmmm, me neither.

    >
    > G
     
    werasm, Nov 3, 2005
    #7
  8. cayblood

    werasm Guest

    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::eek:stream_iterator<int>(std::cout) );

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

    Regards,

    W
     
    werasm, Nov 3, 2005
    #8
  9. cayblood

    Guest

    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, 2005
    #9
  10. cayblood

    Neil Cerutti Guest

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


    vector<T>::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<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());

    --
    Neil Cerutti
     
    Neil Cerutti, Nov 3, 2005
    #10
  11. 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
     
    Valentin.Samko, Nov 3, 2005
    #11
  12. 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
     
    Valentin.Samko, Nov 3, 2005
    #12
  13. cayblood

    cayblood Guest

    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());
     
    cayblood, Nov 4, 2005
    #13
  14. cayblood

    werasm Guest

    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<int>(), 2))
    > , v.end());


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

    W
    >
    > --
    > Neil Cerutti
     
    werasm, Nov 4, 2005
    #14
  15. cayblood

    Neil Cerutti Guest

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

    --
    Neil Cerutti
     
    Neil Cerutti, Nov 4, 2005
    #15
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. unishippers.suckfeed.newshosting.com

    XSD - proper way to express group of elements...

    unishippers.suckfeed.newshosting.com, Nov 3, 2004, in forum: XML
    Replies:
    1
    Views:
    354
    Priscilla Walmsley
    Nov 4, 2004
  2. david wolf

    delete elements in vector

    david wolf, Apr 19, 2005, in forum: C++
    Replies:
    9
    Views:
    12,369
    Victor Bazarov
    May 11, 2005
  3. Replies:
    8
    Views:
    1,930
    Csaba
    Feb 18, 2006
  4. zl2k
    Replies:
    27
    Views:
    1,590
    Francesco S. Carta
    Sep 7, 2010
  5. cassiope
    Replies:
    4
    Views:
    192
    cassiope
    Nov 28, 2011
Loading...

Share This Page