Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Which of the following to you find more readable? The hand-written loop:

for (std::vector<Foo>::const_iterator it = vec.begin(); it != vec.end(); ++it)
{
    bar.process(*it);
}

Or the algorithm invocation:

#include <algorithm>
#include <functional>

std::for_each(vec.begin(), vec.end(),
              std::bind1st(std::mem_fun_ref(&Bar::process), bar));

I wonder if std::for_each is really worth it, given such a simple example already requires so much code.

What are your thoughts on this matter?

share|improve this question
2  
For C++, the answer is pretty obvious. But in other languages, both are about equal (e.g. Python: map(bar.process, vec), although map for side effects is discouraged and list comprehensions/generator expressions are recommended over map). –  delnan Jan 15 '11 at 11:03
5  
And then there is also BOOST_FOREACH ... –  Benjamin Bannier Jan 15 '11 at 13:40
    
So, the item in the effective STL did not convince you? stackoverflow.com/questions/135129/… –  Job Feb 4 '12 at 17:27

11 Answers 11

up vote 18 down vote accepted

There's a reason that lambdas were introduced, and it's because even the Standard Commitee recognizes that the second form sucks. Use the first form, until you get C++0x and lambda support.

share|improve this answer
2  
The same reasoning can be used to justify the second form: the Standard Committee recognized that it was so much superior that they introduced lambdas to make it even better. :-p (+1 nevertheless) –  Konrad Rudolph Jan 15 '11 at 10:40
    
I don;t think the second for is that bad. But it can be better. For example you deliberately made the Bar object harder to use by not adding operator(). By using this it greatly simplifies the call point in the loop and makes the code tidy. –  Loki Astari Jan 15 '11 at 15:27
    
Note: lambda support was added because of two major complaints. 1) The standard boilerplate needed to pass parameters to functors. 2) Not having the code at the call point. Your code satisfies neither of these as you are just calling a method. So 1) no extra code for passing parameters 2) The code is already not at the call point so lambda will not improve the code maintainability. So yes lambda are a great addition. This is not a good argument for them. –  Loki Astari Jan 15 '11 at 15:33

Always use the variant that describes best what you intend to do. That is

For each element x in vec, do bar.process(x).

Now, let's examine the examples:

std::for_each(vec.begin(), vec.end(),
              std::bind1st(std::mem_fun_ref(&Bar::process), bar));

We have a for_each there, too - yippeh. We have the [begin; end) range we want to operate on.

In principle, the algorithm was much more explicit and thus preferrable over any hand-written implementation. But then ... Binders? Memfun? Basically C++ interna of how to get hold of a member function? For my task, I don't care about them! Neither do I want to suffer from this verbose, creepy syntax.

Now the other possibility:

for (std::vector<Foo>::const_iterator it = vec.begin(); it != vec.end(); ++it)
{
    bar.process(*it);
}

Granted, this is a common pattern to recognize, but ... creating iterators, looping, incrementing, dereferencing. These too are all things I don't care for in order to get my task done.

Admittedly, it looks waay better than the first solution (at least, the loop body is flexible and quite explicit), but still, it's not really that great. We'll use this one if we had no better possibility, but maybe we have ...

A better way?

Now back to for_each. Wouldn't it be great to literally say for_each and be flexible in the operation that is to be done, too? Fortunately, since C++0x lambdas, we are

for_each(v.begin(), v.end(), [&](const Foo& x) { bar.process(x); })

Now that we've found an abstract, generic solution to many related situations, it's worth noting that in this particular case, there is an absolute #1 favorite:

foreach(const Foo& x, vec) bar.process(x);

It really can't get much clearer than that. Thankfully, C++0x get's a similar syntax built-in!

share|improve this answer
1  
Said "similar syntax" being for (const Foo& x : vec) bar.process(x);, or using const auto& if you like. –  Jon Purdy Jan 15 '11 at 18:58
    
const auto& is possible? Didn't know that - great info! –  Dario Jan 15 '11 at 19:05

Because this is so unreadable?

for (unsigned int i=0;i<vec.size();i++) {
{
    bar.process(vec[i]);
}
share|improve this answer
4  
Sorry, but for some reason it says "censored" where I believe your code should be. –  muntoo Jan 15 '11 at 23:07
6  
Your code gives a compiler warning, because comparing an int with a size_t is dangerous. Also, indexing doesn't work with every container, for example std::set. –  FredOverflow Jan 24 '11 at 19:00
1  
This code will only work for containers with an indexing operator, of which there are few. It ties the algorithm down unnessarily - what if a map<> would suit better? The original for loop and for_each would be unaffected. –  JBRWilkinson Feb 5 '12 at 14:33
2  
And how many times do you design code for a vector and then decide to swap it for a map? As if designing for that was the languages priority. –  Martin Beckett Feb 5 '12 at 16:22
1  
Iterating over collections by index is discouraged because some collections have poor performance on indexing, whereas all collections behave nicely when using an iterator. –  kbok Nov 20 '12 at 15:51

in general, the first form is readable by pretty much anyone that knows what a for-loop is, no matter wat background they have.

also in general, the second one is not that readable at all: easy enough figuring what for_each is doing, but if you've never seen std::bind1st(std::mem_fun_ref I can imagine it's hard to understand.

share|improve this answer

Actually, even with C++0x, I am not sure for_each will get much love.

for(Foo& foo: vec) { bar.process(foo); }

Is way more readable.

The one thing I dislike about algorithms (in C++) is them reasoning on iterators, makes for very verbose statements.

share|improve this answer

If you had written bar as a functor then it would be a lot simpler:

// custom functor
class Bar
{    public: void operator()(Foo& value) { /* STUFF */ }
};


std::for_each(vec.begin(), vec.end(), Bar());

Here the code is quite readable.

share|improve this answer
1  
Its quite readable aside from the fact that you just wrote a functor. –  Winston Ewert Feb 7 '11 at 3:06
    
See here for why I think this code is worse. –  sbi Nov 20 '12 at 16:15

I prefer the latter because it's neat and clean. It's actually part of many other language but in C++, it's part of library. It's not really matters.

share|improve this answer

Now this issue actually is not specific to C++, I would claim.

The thing is, passing some functor into yet another function is not meant to replace loops.
It is supposed to simplify certain patterns, that become very natural with functional programming, such as visitor and observer, just to name two, that come to my mind immediately.
In languages that lack first order functions (Java probably is the best example), such approaches always require implementing some given interface, which is quite verbous and redundant.

A common use I see a lot in other languages would be:

someCollection.forEach(someUnaryFunctor);

The advantage of this is, that you need not know how someCollection is actually implemented or what someUnaryFunctor does. All you need to know is, that its forEach method will iterate all of the collection's elements, passing them to the given function.

Personally, if you're in the position, to have all information about the data structure you want to iterate over and all information about what you want to do in every iteration step, then a functional approach is overcomplicating things, especially in a language, where this is appearently quite tedious.

Also, you should bear in mind, that the functional approach is slower, because you have a lot of calls, which come at a certain cost, that you don't have in a for loop.

share|improve this answer
    
"Also, you should bear in mind, that the functional approach is slower, because you have a lot of calls, which come at a certain cost, that you don't have in a for loop."? This statement is unsupported. The functional approach is not necessarily slower, especially since there may be optimizations under the hood. And even if you completely understand your loop, it will probably look cleaner in its more abstract form. –  Andres F. Feb 7 '11 at 17:52
    
@Andres F: So this looks cleaner? std::for_each(vec.begin(), vec.end(), std::bind1st(std::mem_fun_ref(&Bar::process), bar)); And it is either slower at runtime or compile time (if you're lucky). I don't see why replacing flow control structures with function calls makes code better. About any alternative presented here is more readable. –  back2dos Feb 8 '11 at 12:05

I think the problem here is that for_each isn't really an algorithm, it is just a different (and usually inferior way) to write a hand written loop. from this perspective it should be the least used standard algorithm, and yes you probably might as well use a for loop. however the advice in the title still stands as there are several standard algorithms tailored to more specifc uses.

Where there is an algorithm that more tightly does what you want then yes you should prefer algorithms over hand written loops. obvious examples here are sorting with sort or stable_sort or searching with lower_bound, upper_bound or equal_range, but most of them have some situation in which they are preferable to use over a hand coded loop.

share|improve this answer

For this small code snippet, both are equaly readable to me. In general, I find manual loops error-prone and prefer using algorithms, but it really depends on a concrete situation.

share|improve this answer

More & more I'm thinking Torvalds was right about C++. I've programmed C++ for nearly 15 years, and Perl for 10, and I've got to say that line noise in C++ is much worse than Perl... Even worse now that they've added lambdas. If you want functional, use Python, people.

share|improve this answer
    
+1 for "line noise". –  John R. Strohm Feb 7 '11 at 7:56

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.