STL for_each complaining about argument list

Question

As part of a homework assignment, we are supposed to map the occurrence of each character in a map. Our function is supposed to use std::for_each and pass in the character to be evaluated.

My function is:

std::for_each(document_.begin(), 
              document_.end(), 
              std::mem_fun(&CharStatistics::fillMap));

document_ is a string, and the fillMap function is defined like

void CharStatistics::fillMap(char ch)
{
    ch = tolower(ch);
    ++chars_.find(ch)->second;
}

chars_ is declared as std::map<char, unsigned int> chars_;.

I figure this should work, but the compiler is complaining

error C2064: term does not evaluate to a function taking 1 arguments

Which confuses me, because when I look at the argument list

_Fn1=std::mem_fun1_t<void,CharStatistics,char>,
1>            _Elem=char,
1>            _Traits=std::char_traits<char>,
1>            _Alloc=std::allocator<char>,
1>            _Result=void,
1>            _Ty=CharStatistics,
1>            _Arg=char,
1>            _InIt=std::_String_iterator<char,std::char_traits<char>,std::allocator<char>>

it looks fine to me. _Elem is a char and my function accepts a char. The iterator is nothing else than a char *

What am I doing wrong?

Answer 1

CharStatistics::fillMap is not a function taking 1 argument. it's member function and so it has implicit first argument - pointer to class instance.

in code:

std::for_each(document_.begin(), 
              document_.end(), 
              std::mem_fun(&CharStatistics::fillMap));

for_each don't know on which instance you'd like to call CharStatistics::fillMap, you didn't specify it. you need to bind it with any CharStatistics instance, e.g.:

std::bind1st(std::mem_fun(&CharStatistics::fillMap), &char_statistics_instance)

Answer 2

document_ is a collection of characters?

But the function is a member function of CharStatistics! Presumably you are calling this from a member function of CharStatistics. In that case you can use boost::bind to solve it if that is allowed:

std::for_each( document_.begin(), document_.end(), 
     boost::bind( &CharStatistics::fillMap, this, _1 );

You could use std::bind1st on "this" which is more complex as you still need mem_fun

std::for_each( document_.begin(), document_.end(), 
      std::bind1st( std::mem_fun(&CharStatistics::fillMap), this ) );

which actually is horribly complex looking. That is why the new bind is so much better!

If you are not allowed to use boost::bind and you don't like the mem_fun solution, write your own functor that overloads operator() to take a char. Like this:

struct CharStatsFunctor
{
   typedef std::map< char, size_t > map_type;
   map_type & mapToFill;
   explicit CharStatsFunctor( map_type & m ) : mapToFill( m ) {}

   void operator()(char ch ) const
   {
       ++mapToFill[ ::tolower( ch ) ];
   }
};

In the loop call

std::for_each( document_.begin(), document_.end(), CharStatsFunctor( chars_ ) );

Note there is an error in your fillMap function. The solution I gave will work.

Answer 3

Basically what is wrong is that your container has a value type char, and for_each expects a function which takes an argument of char, but std::mem_fun(&CharStatistics::fillMap) evaluates to a function object which takes an instance of CharStatistics (on which it will then call fillMap)

Why not simply change your function:

void CharStatistics::fillMap(std::string const& str)
{
  std::string::const_iterator it(str.begin()), end(str.end());
  for(; it != end; ++it)
    ++chars_.find(tolower(*it))->second;
}

Answer 4

If CharStatistics::fillMap is not a static member function, then you need to bind the call to an instance:

CharStatistics instance;
std::for_each(
     document_.begin(),
     document_.end(),
     std::bind1st(
         &CharStatistics::fillMap,
         &instance
     )
);

Further, if it's not a static member function, then it actually has two arguments. The first is the implicit this pointer, and the second is the char. So you have to bind two arguments, using boost::bind (or std::bind if you are on C++0x):

CharStatistics instance;
std::for_each(
     document_.begin(),
     document_.end(),
     boost::bind(
         &CharStatistics::fillMap,
         &instance,
         _1
     )
);

for_each should now see the bind2nd instance as a function object taking one argument (_1), and the instance will be passed automatically.