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.

Quote:

---

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::plus:

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?

Quote:

---

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.

Quote:

---

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.

Quote:

---

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.

Quote:

---

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

---

I agree. for_each is like a fallback when you want to iterate over a range but nothing else comes to mind.

Quote:

---

transform being the non-destructive version.

---

It can be destructive too, if the output range is the input range.

Quote:

---

Originally Posted by CornedBee

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

---

Thanks, indeed I was using gcc. I thought it was the same as g++ and it was easier to type. guess I was wrong. (-:

g++ is the same as gcc, except that it has some default options for C++. The most important among them is -lstdc++.

Quote:

---

Originally Posted by laserlight

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.

---

Why? because it is easier to compile?
I want to adopt a proper style. Is it really the common/good practice to make functions/functors which are passed to STL, free?
Surely it should be free if it can be reused but what about functions/functors that are specific to a class container?

Quote:

---

In this case, the function does not access any members of the class, so it really is best as a free function.

---

Never mind this case. This is not a real example. just a small program to try to figure out how to pass functions/functors to STL algorithms.

Quote:

---

Why? because it is easier to compile?

---

No. I suggest reading GotW #84: Monoliths "Unstrung".

Quote:

---

I want to adopt a proper style. Is it really the common/good practice to make functions/functors which are passed to STL, free?
Surely it should be free if it can be reused but what about functions/functors that are specific to a class container?

---

Refer to GotW #84.

Quote:

---

Originally Posted by laserlight

No. I suggest reading GotW #84: Monoliths "Unstrung".

---

Thanks, that was very interesting.

But how do we prevent namespace pollution?
Should I put all those nonmembers in some header and have it included when needed?
I know this might seem a silly question, but I really like to know.

Quote:

---

Originally Posted by kroiz

But how do we prevent namespace pollution?

---

That article has a paragraph about that:

Quote:

---

Namespace pollution. Because empty() is such a common name, putting it at namespace scope risks namespace pollution -- after all, will every function named empty() want to have exactly these semantics? This argument is also valid to a point, but weakened in two ways: First, by noting that encouraging consistent semantics for functions is a Good Thing; and second, by noticing that overload resolution has turned out to be a very good tool for disambiguation, and namespace pollution has never been as big a problem as some have made it out to be in the past. Really, by putting all the names in one place and sharing implementations, we're not polluting that one namespace as much as we're sanitizing the functions by gathering them up and helping to avoid the gratuitous and needless incompatibilities that Meyers mentions (see above).

---

Quote:

---

Originally Posted by kroiz

Should I put all those nonmembers in some header and have it included when needed?

---

The free functions are still part of the public interface of your class, so it makes sense to put them in the same header as the class. Of course, if you are providing them as some optional extension to the basic functionality provided by your class, then it would be better to separate them into another header.

Got it. Thanks.