Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why can't for_each modify its functor argument?

Tags:

c++

foreach

stl

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
like image 632
smerlin Avatar asked Jan 20 '10 14:01

smerlin


2 Answers

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.

like image 74
James McNellis Avatar answered Nov 08 '22 07:11

James McNellis


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; });
like image 10
GManNickG Avatar answered Nov 08 '22 07:11

GManNickG