IsaIkari
IsaIkari

Reputation: 1154

Why passing struct function to for_each loop does not change the struct attributes

I'm new to C++ and I'm learning with C++20. I'm trying on a struct function, which is to wrap a function in a struct, while we can claim local attributes in this struct.

The thing is that when I pass this struct function to a for_each function, it does not work.

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

using namespace std;

struct accumulateAmount{
    int total_amount;
    accumulateAmount() { total_amount = 100 ;} //constructor
    void operator()(int num){
        total_amount += num;
    }
};


int main(){ 
    vector<int> nums{1,2,3,4,5};
    accumulateAmount acctor;
    for_each(nums.begin(), nums.end(), acctor);
    cout << acctor.total_amount << endl;
    return 0;
    
}

The output is 100. It does not realize the accumulator functionality.

While if I change the loop from for_each to ordinary for loop as the following:

for (int i = 0; i < nums.size(); i++){
      acctor(nums[i]);
}
 

It works.

So I wonder if it's because 'for_each' encompasses parallel computing hence for each int in the vector, we are using independent functions on them?

Upvotes: 0

Views: 414

Answers (2)

sweenish
sweenish

Reputation: 5202

The quickest fix:

acctor = std::for_each(nums.begin(), nums.end(), accumulateAmount());

But rather than spinning your own functor from scratch, use C++'s lambdas (C++11+).

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

int main(){
    std::vector<int> nums{1,2,3,4,5};
    int val = 100;
    std::for_each(nums.begin(), nums.end(), [&val](int n) { return val += n; });
    std::cout << val << '\n';

    return 0;
}

Some quick notes on the lambda:

  • [] is where you can 'capture' variables outside the scope of the lambda, that shouldn't be passed as arguments. In this case I capture val by reference. You are not required to capture anything, but the [] is required.
  • () parameter list, pretty straightforward.
  • {} function body, also straightforward.

As pointed out in a comment to another answer, this specific example is solved even simpler with std::accumulate or std::reduce (Shown in another answer (this one was new to me, and pretty cool)).

#include <iostream>
#include <numeric>
#include <string>
#include <vector>

int main() {
  std::vector<int> nums{1, 2, 3, 4, 5};
  std::cout << std::accumulate(nums.begin(), nums.end(), 100) << '\n';

  return 0;
}

Upvotes: 2

Konrad Rudolph
Konrad Rudolph

Reputation: 545518

std::for_each takes the function by value. So your function gets copied, and std::for_each calls the copy. That’s why your acctor does not get modified.

You can force passing by reference though, by using std::ref:

for_each(nums.begin(), nums.end(), std::ref(acctor));

Alternatively, and perhaps more idiomatically, you can capture the return value of std::for_each:

auto const result = for_each(nums.begin(), nums.end(), accumulateAmount());
std::cout << result.total_amount << "\n";

The good thing about this code is that you don’t even need to introduce a name for acctor: you can pass a temporary and create the function object on the fly. This is nice because it means that you can make all your local objects const.

That said, std::for_each with a mutable function object is absolutely not idiomatic C++. Finding the suitable algorithm isn’t always obvious, but always worth it. In this case, you’d use std::reduce:

auto const result = std::reduce(nums.begin(), nums.end(), 100);
std::cout << result << "\n";

Upvotes: 4

Related Questions