std::for_each, object with operator() overloaded not maintaining state

In trying to answer this question I came up with the following code:

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

class Sizes
{
public:
    void operator() ( std::vector<int> v ) { 
        sizeVec.push_back( v.size() );  
    }
    std::vector<int> sizeVec;
};

void outFunc (int i) {
    std::cout << " " << i;
}
int _tmain(int argc, _TCHAR* argv[])
{
    std::vector<std::vector<int>> twodVec;

    std::vector<int> vec;
    vec.push_back( 6 );
    twodVec.push_back( vec );
    vec.push_back( 3 );
    twodVec.push_back( vec );
    vec.push_back( 8 );
    twodVec.push_back( vec );
    vec.push_back( 3 );
    twodVec.push_back( vec );

    Sizes sizes;
    std::for_each( twodVec.begin(), twodVec.end(), sizes );
    std::for_each( sizes.sizeVec.begin(), sizes.sizeVec.end(), outFunc );

    return 0;
}

Debugging this shows Sizes::operator() being called and the size of sizeVec increasing with each call as expected. However when the second std::fore开发者_如何学运维ach is called the sizeVec is empty... I've created a work around involving passing a vector into Sizes but does anyone know what's going on

---

std::for_each takes a functor by value, not by reference, so the original object is unaffected. You need to do:

sizes = std::for_each( twodVec.begin(), twodVec.end(), sizes );

---

The code snippets shown below are quoted from your code as it was when I wrote this.

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

class Sizes
{
public:
    void operator() ( std::vector<int> v ) { 

v is passed by value, which can be pretty inefficient! Pass it by reference.

        sizeVec.push_back( v.size() );  
    }
    std::vector<int> sizeVec;
};

void outFunc (int i) {
    std::cout << " " << i;
}
int _tmain(int argc, _TCHAR* argv[])

_tmain has never been a valid form of main.

This code will, at best, only compile with Microsoft's compiler.

Also, _tmain has no purpose even with Microsoft's compiler, except for one special case when targeting Windows 9x (not even for targeting Windows 9x in general).

Why are you writing more in order to render your code non-standard and ungrokkable for non-Windows programmers?

Use standard main.

{
    std::vector<std::vector<int>> twodVec;

The >> will probably compile with most modern compilers, due to their support for the upcoming C++0x. But in C++98/C++03 it is invalid. So, still, for portable code write > > (note the space).

    std::vector<int> vec;
    vec.push_back( 6 );
    twodVec.push_back( vec );
    vec.push_back( 3 );
    twodVec.push_back( vec );
    vec.push_back( 8 );
    twodVec.push_back( vec );
    vec.push_back( 3 );
    twodVec.push_back( vec );

    Sizes sizes;
    std::for_each( twodVec.begin(), twodVec.end(), sizes );

sizes can be freely copied here, and is in fact passed by value.

However, std::for_each returns a copy of the final result.

You can assign that back to sizes, even if it is a quite inefficient way of doing things when the functor contains a vector.

    std::for_each( sizes.sizeVec.begin(), sizes.sizeVec.end(), outFunc );

    return 0;

This final return 0; is not necessary with a standard main, since it only expresses the default return value for standard main.

}

Cheers & hth.

---

I would do a couple of things differently:

// (1) struct rather than class for functor (as it contains no state).
struct Sizes
{
    // (2) Keep a reference to the vector
    std::vector<int>&    sizeVec;

    Sizes(std::vector<int>& sizeVec): sizeVec(sizeVec) {}

    // (3) The functor can now be const as the the state is external
    void operator() ( std::vector<int> const& v ) const
    {                              //  ^^^^^^  (4) Pass parameter by const reference
                                   //              This avoids an unnecessary copy.
        sizeVec.push_back( v.size() );  
    }
};

std::for_each( twodVec.begin(), twodVec.end(), Sizes(vec) );
                                         //    ^^^^^^^^^^  Call using temporary
                                         //                No need for a Size variable.

Your second for_each and outFunc() can be replaced with some standard objects:

std::copy(sizes.sizeVec.begin(),
          sizes.sizeVec.end(),
          std::ostream_iterator<int>(std::cout, " ")
         );

A secondary though more obscure note, By keeping state out of the Sizes object makes it easier to convert to C++0x lambda when this arrives next year:

std::for_each( twodVec.begin(), twodVec.end(), Sizes(vec) );

// becomes

std::for_each( twodVec.begin(),
               twodVec.end(), 
               [&vec] ( std::vector<int> const& v ) {  vec.push_back(v.size()); }
             );