Idiomatic C++ for remove_if

Question

I have this class

class Point2D
{
public:
 bool isValid();
 // ...
private:
 double x_, y_;
};

I have a std::vector< Point2D > and I would like to remove the invalid points, now I do like this:

bool invalid ( const Point2D& p )
{
 return !p.isValid();
}

void f()
{
 std::vector< Point2D > points;
 // fill points
 points.erase( std::remove_if( points.begin(), points.end(), invalid ), points.end() );
 // use valid points
}

Is there a standard way of doing this (beautifully), for example without the need of "duplicating" the functionality of the class method Point2D::isValid?

Maybe using C++11 lambda (I am not very familiar with lambda)?

score 16 · Accepted Answer · answered Jun 07 '11 at 09:25

16

Try this:

points.erase(std::remove_if(points.begin(), 
                            points.end(),
                            std::not1(std::mem_fun_ref(&Point2D::isValid))), 
             points.end());

answered Jun 07 '11 at 09:25

n. m. could be an AI

112,515
14
128
243

score 11 · Answer 2 · answered Jun 07 '11 at 09:19

11

Not totally standard but nearly : you can use boost::bind and do the following

points.erase( std::remove_if( points.begin(), points.end(),
  !boost::bind(&Point2D::isValid, _1 )), points.end() );

By the way you should declare the isValid method const.

answered Jun 07 '11 at 09:19

Randall Flagg

310
3
8

+1 I prefer using bind over lambdas in this case, since you don't need to duplicate the signature. Since the OP seems to allow C++0x stuff, std::bind is probably a better choice tho. – ltjax Jun 07 '11 at 09:24
2

I'm curious about what you mean by "won't really work". It's a documented feature (cf http://www.boost.org/doc/libs/1_46_1/libs/bind/bind.html) and I've used it previously without trouble – Randall Flagg Jun 07 '11 at 09:28
2

@ltjax This feature is not present for `std::bind`, it's Boost only. – Luc Danton Jun 07 '11 at 11:48

Xeo · Answer 3 · 2011-06-07T09:29:07.573

10

The lambda version won't be any cleaner either, but it has another important advantage: locality. You see the code where you use it:

points.erase( std::remove_if( points.begin(), points.end(),
              [](const Point2D& p){
                return !p.isValid();
              }), points.end() );

Note, that you need to change isValid to make it a const function, otherwise you can't call it on a reference-to-const (const Point2D&).
Another option would be to implement operator! for your class:

class Point2D{
  // ... as before
public:
  bool isValid() const;

  bool operator!() const{
    return !isValid();
  }
};

Note, both functions are const. Now you could implement a generic negating functor:

struct negate{
  template<class T>
  bool operator()(T const& t){
    return !t;
  }
};

And use that:

points.erase( std::remove_if( points.begin(), points.end(), negate()), points.end() );

edited Jun 07 '11 at 09:29

answered Jun 07 '11 at 09:16

Xeo

129,499
52
291
397

6

Even though the answer is correct, I would advise *against* abusing operator overloading. At the very least, if used, it should be in conjunction with the Safe Bool Idiom. – Matthieu M. Jun 07 '11 at 09:33
@Matthieu M. +1 I agree, I do not like very much the operator overloading and I find "negate" not so readable in the remove_if – Alessandro Jacopson Jun 07 '11 at 17:56
1

To extend on the comment of Matthieu M., see http://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool The c++11 standard gives an elegant solution by allowing explicit conversion operators. – swalog Jan 27 '14 at 21:03

score 6 · Answer 4 · answered Jun 07 '11 at 09:26

You can do what you want using a combination of std::mem_fun_ref and std::not1:

points.erase( std::remove_if( points.begin(), points.end(),
                              std::not1( std::mem_fun_ref( &Point2D::isValid ) ) ),
              points.end() );

For what it's worth, the only "idiomatic" part about this is the erase-remove idiom.

score 5 · Answer 5 · answered Jun 07 '11 at 10:03

5

If Boost is OK for you, use what @Randall Flagg suggested together with boost::remove_erase_if:

boost::remove_erase_if(points, !boost::bind(&Point2D::isValid, _1));

answered Jun 07 '11 at 10:03

Yakov Galka

70,775
16
139
220

1

@monkey: the first argument passed when the functor is called. In this case it is the `point` in consideration, which I want to bind to `this` so that bind is roughly equivalent to `!_1->Point2D::isValid()`. – Yakov Galka Oct 08 '12 at 07:47

score 2 · Answer 6 · answered Jun 07 '11 at 09:17

2

I think you are looking for not1

Edit: Looking at your example closer i don't think you can do it any other way, since isValid() is a member function.

answered Jun 07 '11 at 09:17

RedX

14,749
1
53
76

That was my immediate reaction too! But I don't think it's directly applicable here, because the OP wants to call a member function on the elements. – Oliver Charlesworth Jun 07 '11 at 09:18
2

I think you can use `not1(mem_fun_ref(&Point2D::isValid))`. – hammar Jun 07 '11 at 09:21

Idiomatic C++ for remove_if

6 Answers6

Linked