Replacing for loop with for_each or accumulate

S

Skirmish

I'd like to replace the following 5 lines of code with a std::for_each
or std::accumulate

bool retval = true;
for (EntityIter it = m_entities.begin(); it != m_entities.end(); +
+it)
{
retval &= (*it)->Release();
}
return retval;

Where Release is declared: bool Entity::Release();

I was hoping something along the lines of:
return std::accumulate(m_entities.begin(), m_entities.end(),
boost::bind(&Graphics::Entity::Release));
or
return std::for_each(m_entities.begin(), m_entities.end(),
std::mem_fun<bool>(&Graphics::Entity::Release));
would do the trick, but both generate wierd and wonderful errors (I
can post these on request).

This is a question of curiosity rather than necessity, but any help
would be greatly appreciated.

Cheers
Adam
 
F

foothompson

I'd like to replace the following 5 lines of code with a std::for_each
or std::accumulate

bool retval = true;
for (EntityIter it = m_entities.begin(); it != m_entities.end(); +
+it)
{
retval &= (*it)->Release();
}
return retval;

Where Release is declared: bool Entity::Release();

I was hoping something along the lines of:
return std::accumulate(m_entities.begin(), m_entities.end(),
boost::bind(&Graphics::Entity::Release));
or
return std::for_each(m_entities.begin(), m_entities.end(),
std::mem_fun<bool>(&Graphics::Entity::Release));
would do the trick, but both generate wierd and wonderful errors (I
can post these on request).

This is a question of curiosity rather than necessity, but any help
would be greatly appreciated.

Cheers
Adam

One issue you might be having is that std::mem_fun might only works
with raw pointers. If that is the problem, then try using
boost::mem_fn instead. Otherwise, provide more info.
 
S

Skirmish

Ok I have found the solution I was seeking:
return accumulate(m_entities.begin(), m_entities.end(), true,
boost::bind(std::logical_and<bool>(), _1,
boost::bind(&Entity::Release, _2)));

But it isn't really clearer then the expanded 5 lines of the original,
and from talking with a colleague it appears there might be overhead
in boost::bind so there probably isn't a good performance reason to go
with the latter (though I am too lazy to profile each version).

What does everyone else think?
 
K

kwikius

I'd like to replace the following 5 lines of code with a std::for_each
or std::accumulate

bool retval = true;
for (EntityIter it = m_entities.begin(); it != m_entities.end(); +
+it)
{
retval &= (*it)->Release();
}
return retval;

Where Release is declared: bool Entity::Release();

I was hoping something along the lines of:
return std::accumulate(m_entities.begin(), m_entities.end(),
boost::bind(&Graphics::Entity::Release));
or
return std::for_each(m_entities.begin(), m_entities.end(),
std::mem_fun<bool>(&Graphics::Entity::Release));
would do the trick, but both generate wierd and wonderful errors (I
can post these on request).

This is a question of curiosity rather than necessity, but any help
would be greatly appreciated.

I'm pretty sure that you need accumulate, but the problem is that
accumulate requires to be passed a function that takes 2 arguments,
the first being the previous accumulated result. for_each (IIRC)
returns void so its not much use. IMO that means that you need either
an overloaded member with suitable args or another entity. It may not
be politically correct but I find its easier just to write a custom
functor than to wrestle with memfun. Its also much more legible IMO.
However, from the following you can probably figure how to do it the
proper way with mem_fun:

#include <numeric>
#include <vector>

struct entity{
entity(bool b):can(b){}
bool release(){return can;}

private:
bool can;
};

struct release{
bool operator()(bool b, entity * p) const
{
return b & p->release();
}
};

#include <iostream>

int main()
{
std::vector<entity*> entities;

entity e1(true),e2(true);
//add some good ones...
entities.push_back(&e1);
entities.push_back(&e2);

bool released1 = std::accumulate(
entities.begin(), entities.end(),true, release()
);
//should be ok
std::cout << "success1 = " << released1 <<'\n';

// add a dud..
entity e3(false);
entities.push_back(&e3);
bool released2 = std::accumulate(
entities.begin(), entities.end(),true, release()
);

std::cout << "success2 = " << released2 <<'\n';
}

regards
Andy Little
 
J

James Kanze

Would it be too much to ask why? If the code clearly expresses
your intent, why should you want to replace it with something
else?

Actually, if you want to replace it, find_if is more what you
need:

return std::find_if( ... ) == m_entities.end() ;

In this case, using boost:bind to create the predicate should
really result in something cleaner and more readable.
I'm pretty sure that you need accumulate,

I'm not sure at all. Once retval becomes false, it stays false,
so what he has implemented is basically nothing more than a
linear search which takes more time than necessary.
but the problem is that
accumulate requires to be passed a function that takes 2 arguments,
the first being the previous accumulated result. for_each (IIRC)
returns void so its not much use.

for_each returns a copy of the functional object. The exact
words don't say it, but the obvious implementation results in
all calls to the functional object going to the same object, and
a copy of that object being returned. I rather suspect that the
intent was that it should be implemented that way.
IMO that means that you need either
an overloaded member with suitable args or another entity. It may not
be politically correct but I find its easier just to write a custom
functor than to wrestle with memfun.

Certainly. And it's even easier not to bother with for_each at
all, unless that functional object has more general use, or
improves the program in some other way.
 
K

kwikius

Would it be too much to ask why? If the code clearly expresses
your intent, why should you want to replace it with something
else?

A steam engine works well enough too :)

I guess the real answer is that functional style programmming can,
when you hit the sweet spot, be extremely satisfying compared to
loops. It is somewhat elusive in C++ however, which is not helped by
the requirement to use explicit iterators and lack of 'auto'. As I
understand these problems are being addressed in C++0x, then I expect
there will be more interest in, and use of functional style
programming in C++.

regards
Andy Little
 
J

James Kanze

A steam engine works well enough too :)
I guess the real answer is that functional style programmming can,
when you hit the sweet spot, be extremely satisfying compared to
loops.

I like it too, but...
It is somewhat elusive in C++ however, which is not helped by
the requirement to use explicit iterators and lack of 'auto'. As I
understand these problems are being addressed in C++0x, then I expect
there will be more interest in, and use of functional style
programming in C++.

To be used effectively in such cases, you need support for
lambda. I know that it is (or was) being discussed for C++0x,
but I don't know what the current status concerning it is. In
the absence of lambda, if the operation is something totally
dependent on the context of the function, and not generally
applicable, trying to factor it out into a functional object
(which must be defined outside the function if it is to be used
to instantiate a template) adds complication and reduces
readability.

Note that I'm not saying that you shouldn't give the idea some
thought. Very often, some sort of generic predicate or
functional object does make sense, and giving a name to the
operation can improve readability. But you shouldn't try to use
accumulate or for_each just to use them. The choice should be
motivated.
 
K

kwikius

I like it too, but...


To be used effectively in such cases, you need support for
lambda. I know that it is (or was) being discussed for C++0x,
but I don't know what the current status concerning it is.

There has been some recent activity in the lambda proposals, N2329
dated 24/06/2007, which has I believe simplified useage and
implementation problems and brought them a step closer to reality.
In
the absence of lambda, if the operation is something totally
dependent on the context of the function, and not generally
applicable, trying to factor it out into a functional object
(which must be defined outside the function if it is to be used
to instantiate a template) adds complication and reduces
readability.

I'm not sure. Factoring code out into functions usually helps
legibility assuming the names are suitable. Imagine a world where
there were no functions and everything had to be declared inline.

One issue with lambda I think is that it encourages you to write
functions inline. There are 2 consequences, the first being that
'higher' functions get longer and the second that you are more likely
to end up reinventing the wheel, or have many nearly similar functions
but not quite the same scattered around to create maintenance
problems.

This is actually part of the attraction of functional style
programming for me, which is that it encourages you to think in terms
of reusable components (often with heavy use of templates). If there
is a problem in a function, it can be fixed in one place.

Its interesting in regard to the related thread "std::string and case
insensitive comparison". In there now are nearly all the components
to write a very useful functional style comparison function.

My choice would now be to use my string_pair_iterator( but try to sort
out the end condition more eficiently), then reread the part of the
discussion on to_lower and pick the best version of that, and finally
use find_if ( instead of accumulate or my custom true_for_each
function) as suggested up this thread by you I believe. The
combination looks like it should should produce an efficient
comparison function.

It took a fair amount of input to bring it all together, but it is
made from some discrete components which are reuseable and can be
recombined in many ways. aka generic programming.
Note that I'm not saying that you shouldn't give the idea some
thought. Very often, some sort of generic predicate or
functional object does make sense, and giving a name to the
operation can improve readability. But you shouldn't try to use
accumulate or for_each just to use them. The choice should be
motivated.

The motivation is that once you have solved a problem once, you don't
need to revisit it, whereas an explicit loop is a revisit each time.
Also explicit loops are inline code, which can perhaps expose low
level details to end up as a maintenance problem. I know that whenever
I write a loop I do think, I wish I could do this more elegantly..

I don't remember the exact quote or who said it, but it goes something
like.. "Apologies for writing so much. I didnt have enough time to be
brief". That is the motivation in trying to find more elegant
solutions. I would have thought its a pretty common goal...

Of course this approach doesnt help programmers paid by lines of code
written :)

regards
Andy Little
 
?

=?iso-8859-1?q?Kirit_S=E6lensminde?=

I guess the real answer is that functional style programmming can,
when you hit the sweet spot, be extremely satisfying compared to
loops. It is somewhat elusive in C++ however, which is not helped by
the requirement to use explicit iterators and lack of 'auto'. As I
understand these problems are being addressed in C++0x, then I expect
there will be more interest in, and use of functional style
programming in C++.

I've been doing a lot of playing around with functional programming in
C++ just recently as well. I guess you already know about
Boost.Function and Boost.Lambda - both very useful.

For those that don't, with Boost.Lambda you can create anonymous
lambda expressions and Boost.Function provides handy syntax for
passing functions about much less verbosely than the normal C++
syntax.


K
 
J

James Kanze

There has been some recent activity in the lambda proposals, N2329
dated 24/06/2007, which has I believe simplified useage and
implementation problems and brought them a step closer to reality.

That's good news. I'd heard that a lot of proposals were being
shelved, because the committee absolutely wants it to be C++0x,
and not C++1x (which means they'll have to have a CD very, very
soon now).
I'm not sure. Factoring code out into functions usually helps
legibility assuming the names are suitable.

Factoring code out into functions is usually good. Forcing a
single line of code out into a 10 line class isn't necessarily a
good thing, especially if the only reasonable name for that line
of code was the name of the function which contained it. The
problem isn't that you can factor the code out; the problem is
that you absolutely have to, even in the cases where it isn't a
good thing.
Imagine a world where
there were no functions and everything had to be declared inline.
One issue with lambda I think is that it encourages you to write
functions inline. There are 2 consequences, the first being that
'higher' functions get longer and the second that you are more likely
to end up reinventing the wheel, or have many nearly similar functions
but not quite the same scattered around to create maintenance
problems.

I'd certainly agree that it's a feature which can easily be
abused.

[...]
The motivation is that once you have solved a problem once, you don't
need to revisit it, whereas an explicit loop is a revisit each time.
Also explicit loops are inline code, which can perhaps expose low
level details to end up as a maintenance problem. I know that whenever
I write a loop I do think, I wish I could do this more elegantly..

Even when you're implementing std::find?

If the loop body is more than a couple of lines, I agree. Ditto
for the then or else parts of an if, or a case in a switch. But
for things like linear search, a loop doesn't bother me.
I don't remember the exact quote or who said it, but it goes something
like.. "Apologies for writing so much. I didnt have enough time to be
brief". That is the motivation in trying to find more elegant
solutions. I would have thought its a pretty common goal...
Of course this approach doesnt help programmers paid by lines of code
written :)

It doesn't help, either, if you only use the functional object
once (unless you're paid by lines of code). In real life, there
are a lot of loops in which you do something very application
specific. Maybe just call a specific function on each of the
objects in the container (but with an argument that was passed
as an argument to your function). Maybe you call a specific
function only if a certain condition is met. (Maybe we need a
for_each_if function in <algorithm>.)

With lambda, I could see writing something like:

void
C::f( T1 a, T2 b )
{
std::for_each( o in c :
{ if ( o.g( a ) ) {
o.h( b ) ;
}
} ) ;
}

However, I certainly couldn't see writing a special class
outside of f just to do it. (And I'm not 100% convinced that
the lambda solution is better than a simple loop.)
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,777
Messages
2,569,604
Members
45,227
Latest member
Daniella65

Latest Threads

Top