tags:

views:

315

answers:

3

http://www.cplusplus.com/reference/algorithm/for_each/
Unary function taking an element in the range as argument. This can either be a pointer to a function or an object whose class overloads operator(). Its return value, if any, is ignored.

According to this article, I expected that for_each actually modifies the object given as its third argument, but it seems like for_each operates on a temporary object, and doesn't even modify the object given to it.

So, why is it implemented in that way? It seems much less useful. Or did I misunderstand something and my code below contains errors?

#include <iostream>
#include <vector>
#include <algorithm>

template <class T> struct Multiplicator{
    T mresult;
  public:
    const T& result() const{return mresult;}
    Multiplicator(T init_result = 1){
      mresult = init_result;
    }
    void operator()(T element){
      mresult *= element;
      std::cout << element << " "; // debug print
    }
};

int main()
{
    std::vector<double> vec;
    vec.push_back(1);
    vec.push_back(2);
    vec.push_back(3);
    Multiplicator<double> multiply;
    std::for_each(vec.begin(),vec.end(),multiply);
    std::cout << "\nResult: " << multiply.result() << std::endl;
    return 0;
}

Expected output:

1 2 3 Result: 6

But got following output:

1 2 3 Result: 1
+11  A: 

The function object is taken by value. for_each returns the function object, so if you change it to:

multiply = std::for_each(vec.begin(),vec.end(),multiply);

you get the expected output.

James McNellis
Generally for functors, and because they are supposed to be lightweight and copyable, I cheat and use an attribute that references a variable passed into the constructor. Thus any copy is stricly equivalent and refers to the same item. Of course, it makes initialization a bit awkward (declare variable, pass it to functor, read from variable), but it works fine and does not require any particular implementation propriety (like not discarding the just modified functor).
Matthieu M.
+9  A: 

While James is correct, using std::accumulate with std::multiplies would be more correct, probably:

#include <iostream>
#include <functional>
#include <numeric>
#include <vector>

int main(void)
{
    std::vector<double> vec;
    vec.push_back(1);
    vec.push_back(2);
    vec.push_back(3);

    double result = std::accumulate(vec.begin(), vec.end(),
                                    1.0, std::multiplies<double>());

    std::cout << "\nResult: " << result << std::endl;

}

With your for_each version, you don't really need to copy the functor again, rather:

double result = std::for_each(vec.begin(), vec.end(), multiply).result();

Or C++0x, for fun:

double result = 1;
std::for_each(vec.begin(), vec.end(), [&](double pX){ result *= pX; });
GMan
What is this, "suggest the right tool for the job day?" :-) +1
James McNellis
Thanks for the tip.
smerlin
@James: 8​​​​​​) Unfortunately, the quality of my answers decreases exponentially as the day continues. :p
GMan
A: 

The semantics of For_each dont fit into what you are trying to do. accumulate does exactly what you are trying, use that instead.

Yogesh Arora