using for_each

Hi,
I am having hard time trying to pass a function to a STL algorithm, in this case for_each.
Please, what am I doing wrong?

Code:

#include <vector>
#include <algorithm>

using namespace std;

class A
{
    vector<int> v;
public:
    void incr()
    {
        for_each(v.begin(), v.end(),
            &A.plus );
    }

    void plus(int& t)
    {
        t++;
    }
        
};

int main()
{
    A a;
    a.incr();
    return 0;
}

gcc gives me:

Code:

test.cpp: In member function ‘void A::incr()’:
test.cpp:13: error: expected primary-expression before ‘.’ token

THe error is that you used &A.plus instead of &A::plus. However, the standard generic algorithms do not work with member functions. In some cases you could use mem_fun or mem_fun_ref to convert a member function to a function object, but that will not work here, methinks. It is probably easiest to just make it a free function.

Thanks laserlight, substituting . with :: and switching to free function does make it compile but not link:

Code:

#include <vector>
#include <algorithm>
#include <functional>

using namespace std;

void plusfree(int& t)
{ 
	t++;
}

class A
{
	vector<int> v;
public:
	void incr()
	{
		for_each(v.begin(), v.end(),
			 plusfree );
	}
};

int main()
{
	A a;
	a.incr();
	return 0;
}

Code:

/tmp/cctA1W0u.o: In function `__gnu_cxx::new_allocator<int>::deallocate(int*, unsigned int)':
test.cpp:(.text._ZN9__gnu_cxx13new_allocatorIiE10deallocateEPij[__gnu_cxx::new_allocator<int>::deallocate(int*, unsigned int)]+0xd): undefined reference to `operator delete(void*)'
/tmp/cctA1W0u.o:(.eh_frame+0x12): undefined reference to `__gxx_personality_v0'
collect2: ld returned 1 exit status

Also you right about mem_fun and mem_fun_ref those do not help. (don't know why).

I think that in a large project always making that kind of functions free is not a good style. I think I would just rather not use for_each at all.

That is strange. Your code compiles and links correctly for me here with the MinGW port of g++ 3.4.5 and MSVC8.

I think that in a large project always making that kind of functions free is not a good style. I think I would just rather not use for_each at all.

It depends on what the function does. In general, you should prefer non-member non-friend (i.e. free) functions to member functions or friend functions. In this case, the function does not access any members of the class, so it really is best as a free function.

You're not linking against the standard C++ library. Make sure you call g++, not gcc, to compile the code.

A free function in this case is not so good - it might not get inlined, for one. Better would be a function object.

Code:

template<typename T>
struct increment : std::unary_function<void, T&>
{
  void operator(T& t) { ++t; }
};

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

Or you could use std::transform, std::bind1st and std:lus:

Code:

std::transform(v.begin(), v.end(), v.begin(), std::bind1st(std::plus<int>(), 1));

Or you could use Boost.Bind.

Code:

std::transform(v.begin(), v.end(), v.begin(), boost::bind(std::plus<int>(), _1, 1));

Or Boost.Lambda.

Code:

namespace ll = boost::lambda;

std::transform(v.begin(), v.end(), v.begin(), ll::_1 + 1);

Or again Boost.Lambda, but with for_each.

Code:

std::for_each(v.begin(), v.end(), ++ll::_1);

Oh, the possibilities.

By the way, with C++0x lambdas, the same code will be

Code:

std::for_each(v.begin(), v.end(), [](int &i) { ++i; });

CornedBee: But what is to gain or to loose if we use or don't use for_each at all?

Originally Posted by manav

CornedBee: But what is to gain or to loose if we use or don't use for_each at all?

for_each provides a pretty diverse way to iterate over a container without having a hard-wired definition for what it does.

Your functor could essentially contain a member variable for the sum if you want the sum, or contain another container if you're moving certain eleemnts to a list. I see functors as functions with a higher purposes, they do not have to have an initial purpose, you can give it one later.

You don't gain or lose anything unless it's your purpose to, you can very well iterate using a while or for loop.

Of course, the for-loop is quite ugly.

Code:

for(std::vector<int>::iterator it = v.begin(); it != v.end(); ++it) {
  ++*it;
}

I prefer for_each or transform. (transform over for_each, actually, as it's more explicit in what it does.)

transform being the non-destructive version.

Originally Posted by CornedBee

Of course, the for-loop is quite ugly.

Code:

for(std::vector<int>::iterator it = v.begin(); it != v.end(); ++it) {
  ++*it;
}

I prefer for_each or transform. (transform over for_each, actually, as it's more explicit in what it does.)

This is not ugly at all:

Code:

using namespace std;
typedef vector<int>::iterator iter;

for (iter i = v.begin(); i != v.end(); ++i)
{
        ++*i;
        // or without having to write a new function.
        // *i = someFancyStuff();
}

There's no wrong or right way to do it, and he was stating his personal opinion (though I agree that for is pretty clunky for advanced container techniques) but for_each and transform are specifically tailored for containers, while for statements are used for anything really. Transform also saves you from writing the code to iterate across one or two containers and push_back or add the result in a resulting container.

also, if i = v.end(), you don't have an element.

Originally Posted by CornedBee

By the way, with C++0x lambdas, the same code will be

Code:

std::for_each(v.begin(), v.end(), [](int &i) { ++i; });

They do come? With that nice interface? Wow, I'm going to love it!

And here I thought I'd be escaping SML after this semester was over

But I do kind of like "SURPRISE" functions as in functional languages.

But if you are doing something like a class that relies on information across many containers, it would make sense to have it override the function operator and use for_each and have that function object do it's thing. Anonymous functions seem to be useful for trivial things but any kind of convenient code would best be left for something else.

I don't see any point here. Just complexity for nil usage!
Thumb's Down!

You unconstructive criticism is appreciated.