0

I have a

using namespace std;
typedef vector<Coil*> CoilVec;
CoilVec Coils;

with Coil being a base class for CilCoil and RectCoil, a cilindrical coil and a rectangular coil, respectively. Now I wish to invoke a member function calcField on each Coil pointed to in Coils. This member function is purely virtual in the base class but has been implemented in the derived classes and its declaration looks like this:

virtual TVector3 calcField(const TVector3&);

with TVector3 being the 3D vector class from the RooT library. The idea now is to calculate the field of every Coil in Coils and add them together. Since the argument of calcField (namely a vector to the position in which to calculate the field) would be the same for every call, I would like to use an STL algorithm from the <algorithm> or <numeric> header to do something like this (imagined):

using namespace std;
typedef vector<Coil*>::const_iterator CoilIt;
const TVector3& P(1.,1.,1.); // Let's say we want to know the total field in (1,1,1)
TVector3 B; // Default initialization: (0,0,0)
CoilIt begin = Coils.begin();
CoilIt end = Coils.end();
B = accumulate(begin, end, B, bind2nd(mem_fun(&Coil::calcField), P));

Obviously, since I'm here to ask a question, this doesn't seem to work. So my question is quite simply put: why does this not work and/or how would you go about doing it the right way (within the limits of the STL)?

I get the following error messages trying to compile the above (the file I'm working in is called Interface.cpp, it's third-party code):

In file included from /usr/include/c++/4.5/numeric:62:0,
                from Interface.cpp:7: /usr/include/c++/4.5/bits/stl_numeric.h: In function ‘_Tp std::accumulate(_InputIterator, _InputIterator, _Tp, _BinaryOperation) [with _InputIterator = __gnu_cxx::__normal_iterator<Coil* const*, std::vector<Coil*> >, _Tp = TVector3, _BinaryOperation = std::binder2nd<std::mem_fun1_t<TVector3, Coil, const TVector3&> >]’:
Interface.cpp:289:72: instantiated from here
/usr/include/c++/4.5/bits/stl_numeric.h:150:2: error: no match for call to ‘(std::binder2nd<std::mem_fun1_t<TVector3, Coil, const TVector3&> >) (TVector3&, Coil* const&)’
/usr/include/c++/4.5/backward/binders.h:147:7: note: candidates are: typename _Operation::result_type std::binder2nd<_Operation>::operator()(const typename _Operation::first_argument_type&) const [with _Operation = std::mem_fun1_t<TVector3, Coil, const TVector3&>, typename _Operation::result_type = TVector3, typename _Operation::first_argument_type = Coil*]
/usr/include/c++/4.5/backward/binders.h:153:7: note:                  typename _Operation::result_type std::binder2nd<_Operation>::operator()(typename _Operation::first_argument_type&) const [with _Operation = std::mem_fun1_t<TVector3, Coil, const TVector3&>, typename _Operation::result_type = TVector3, typename _Operation::first_argument_type = Coil*]
Wouter
  • 161
  • 1
  • 6
  • Not going to give this as an answer as you asked for something within the STL, but have you considered the Boost libraries (specifically boost::bind)? I was having the same problem switching over fixed it. – Samuel Breese Mar 18 '12 at 12:19
  • Sounds like you want `for_each`, not `accumulate`. – interjay Mar 18 '12 at 12:21
  • `Coils` is a type, not an object, so `Coils.begin()` doesn't make sense. – fredoverflow Mar 18 '12 at 12:49
  • @chameco: I have considered it, but I'd rather stick to the STL in this case. – Wouter Mar 18 '12 at 13:00
  • @Wouter No, you really shouldn't. The `bind1st` and `bind2nd` have been deprecated for a reason. What you want is impossible with them. – pmr Mar 18 '12 at 13:04
  • @pmr That's too bad, I was hoping to avoid using boost since I'm not yet familiar with it. – Wouter Mar 18 '12 at 13:15

4 Answers4

0

The function passed to std::accumulate() has to be a binary function, each call to the function will be given two parameters (the already accumulated value and the next element of the vector).

The bound function you pass to std::accumulate() only takes one parameter, a Coil*. To be used with accumulate, it would have to take two parameters (Vector3D and Coil*).

sth
  • 222,467
  • 53
  • 283
  • 367
0

The result of your bind2nd call is a unary function that takes a Coli*. std::accumulate expects a binary function that can be called with the accumulator and the current value, i.e. it wants a function that takes (TVector3, Coli*).

It seems like what you're trying to do is transform each Coli* to a TVector3 and then accumulate the results.

You could do this with an explicit std::transform step to fill a vector<TVector3> and then use std::accumulate, or you could write a separate free function that takes (TVector3 t1, TVector3 t2, Coli* c), returns t2 + c->calcField(t1), then use bind1st on that to get your binary accumulator function.

If you want to do it without writing a separate free function, you'll need boost::bind or C++11.

je4d
  • 7,628
  • 32
  • 46
  • This actually compiles! I still have to run some tests to check if it does what I want it to do, but I'm hopeful :) – Wouter Mar 18 '12 at 13:41
0

It's probably not worth the trouble to get your code to compile with mem_fun, bind2nd and friends. I would strongly suggest writing a good old-fashioned loop instead:

for (CoilIt it = coils.begin(); it != coils.end(); ++it)
{
    B += (**it).calcField(P);
}
fredoverflow
  • 256,549
  • 94
  • 388
  • 662
  • This will probably be my temporary solution until I have a deeper look into boost. Still, I'd like to avoid for-loops, even when they appear to be acceptable. – Wouter Mar 18 '12 at 13:23
0

What you want is a nested bind. Those are incredibly hard to do with the deprecated binders. Here is a simplified example with boost::bind:

#include <iostream>
#include <vector>
#include <algorithm>
#include <functional>
#include <numeric>
#include <boost/bind.hpp>

struct A {
  double foo(double d) { return d; }
};

int main()
{
  std::vector<A> vs;
  double d = 0.0;
  d = std::accumulate(begin(vs), end(vs), d, 
                      boost::bind(std::plus<double>(), _1, 
                                  boost::bind(boost::mem_fn(&A::foo), _2, 0.0)));
  return 0;
}

I believe your misunderstanding is that you assume accumulate uses operator+ to accumulate the result of the functor you pass in. In fact the functor is used to accumulate the result. This is based on the general concept of a fold in functional programming.

The functor you pass in must be of the form

Ret func(const T1& a, const T2& b)

where Ret must be convertible to T (the type of the init parameter), T1 must be such that T can be implicitly converted to it and T2 must be such that the result of dereferencing an iterator into the sequence must be implicitly convertible to it.

pmr
  • 58,701
  • 10
  • 113
  • 156
  • I'll have to work up some knowledge of boost to really understand this then. We should be taking a look at the boost library in my C++ course on tuesday. Hopefully that will help clear things up. But as I said in a comment on my question: I was hoping an STL solution was possible, at least for now. – Wouter Mar 18 '12 at 13:21
  • @Wouter C++11 has `std::bind` which is just a standardized version of `boost::bind`. The code above should be nearly 100% compatible with it. If you have a recent compiler, go with it. Then again, you will probably use a lamdba to do this with a recent compiler. – pmr Mar 18 '12 at 13:29