views:

469

answers:

8

Which one of these do you prefer?

foreach(var zombie in zombies)
{
    zombie.ShuffleTowardsSurvivors();
    zombie.EatNearbyBrains();
}

or

zombies.Each(zombie => {
    zombie.ShuffleTowardsSurvivors();
    zombie.EatNearbyBrains();
});
+19  A: 

The first. It's part of the language for a reason.

Personally, I'd only use the second, functional approach to flow control if there is a good reason to do so, such as using Parallel.ForEach in .NET 4. It has many disadvantages, including:

  • It's slower. It's going to introduce a delegate invocation at each element, just like you did foreach (..) { myDelegate(); }
  • It's non-standard, so will be more difficult to understand by most developers
  • If you close over any locals, you're going to force the compiler to make a closure. This can lead to strange issues if there's threading involved, plus adds completely unnecessary bloat to the assembly.

I see no reason to write your own syntax for a flow control construct that already exists in the language.

Reed Copsey
Its actually not slower, see my post.
Igor Zevaka
It's not slower using List<T>.ForEach, but in general, this approach is slower.
Reed Copsey
A: 

I don't prefer either, because of what I consider to be an un-needed use of 'var'. I would write is as:

foreach(Zombie zombie in zombies){
}

But as to the Functional or foreach, for me I most definitely prefer foreach, because there doesn't seem to be a good reason for the latter.

Noon Silk
**Every** use of `var` is un-needed! It's just syntactical sugar. It makes things easier, though. Why type out the type, when the compiler already knows it?
Blorgbeard
Really? You'd prefer to write zombie 3 times?? Like we would't know it's a zombie??
Dave Newman
Blorgbeard: There are some times when it's required, I believe. Dave: Yep, I'd prefer to save `var` for when it's really needed. I'm using a language with explicit types for a reason :) I like to know what is what with just a glance.
Noon Silk
I would write out the type. I know it seems obvious that it should be of type Zombie, but that's what everyone says before they encounter an impossible-to-find bug. It makes the compiler check the types for you, and it makes the code self-documenting. I don't think var is worth it in this case.
Ray Hidayat
Yes, I agree with silky 100%! I only use var when the situation really justifies it.
Ray Hidayat
@Blorgbeard, explain to me how you would store anonymous types without using the `var` keyword? and anonymous types are very useful when combined with LINQ. I agree with Silky however, so +1
Stan R.
Wow, you guys are going to _hate_ the dynamic keyword :)
Dave Newman
@silky: most devs I know only use var when the right hand side is explicitly creating a type, so you can still "know what is what with just a glance". I tend to use descriptive names for my types; I don't need to see it written out twice. Isn't var msgPublisher = new AsyncMessagingServicePublisher() much more readable than AsyncMessagingServicePublisher msgPublisher = new AsyncMessagingServicePublisher() ?
Rob Levine
Rob: I'd probably consider that name a bit too long. Regardless, yes I do think it's more readable (inside an text editor where it will be coloured appropriately).
Noon Silk
@Ray - The compiler does check the type for you, even if you use `var`. The only way using var could lead to a bug that passes the compiler is if a non-Zombie class happened to have a `ShuffleTowardsSurvivors` method, and you happened to put a bunch of instances of that non-Zombie class in a variable called `zombies`.
Joel Mueller
lol at "save var for when it's really needed". Wouldn't you want to save the explicit `Zombie` for when it's really needed? Perhaps I'm biased having spent years coding ruby and javascript, but IMHO most of the time who gives a crap what the type actually is, so long as it's the right OBJECT
Orion Edwards
@Joel - Yes, I understand - that was actually the type of checking I was talking about. I suppose if we're talking about this case specifically, then it is unlikely that another class would have a ShuffleTowardsSurvivors method, and so I'm arguing more for the general rule, as opposed to how this particular situation should be handled.
Ray Hidayat
@Orion - +1 for your comment; as long as the compiler knows what type it is I don't see why I should have to care.
Greg Beech
+1  A: 

This question contains some useful discussion, as well as a link to an MSDN blog post, on the philosophical aspects of the topic.

Sapph
A: 

I'd think the second form would be tougher to optimize, as there's no way for the compiler to unroll the loop any differently for this one call than it does for anybody else's call to the Each method.


Since it was asked, I'll elaborate. The method's implementation is quite liable to be compiled separately from the code that invokes it. This means that the compiler does not know exactly how many loops it is going to have to perform.

If you use the "foreach" form then that information may be avaliable to the compiler when it is creating the code for the loop (it also may not be available, in which case no difference).

For example, if the compiler happens to know (from previous code in the same file) that the list has exactly 20 items in it, it can replace the entire loop with 20 references.

However, when the compiler creates code for the "Each" method off in its source file, it has no idea how big the caller's list is going to be. It has to support any size. The best it can do is try to find some kind of optimum unrolling for its CPU, and add extra code to loop through that and do a proper loop if it is too small for the unrolling. For a typical small loop this might even end up being slower. Of course for small loops you don't care as much....unless they happen to be inside a big loop.

As another poster mentioned, this is (and should be) a secondary concern. The important thing is which is easier to read and/or maintain, but I don't see a huge difference there.

T.E.D.
Why not? In both situations there is an `IEnumerator` with `MoveNext` getting called on it behind the scenes... The main difference is that with the lambda, there's an extra call to the lambda function (which is a tiny private static method somewhere); but the JIT is fairly aggressive at inlining short static methods, so it's probably going to inline it, leading to the exact same thing when all is said and done
Orion Edwards
TBH I'd be shocked if it unrolled any loops with a `foreach` at all. It really only makes sense when you're dealing with old-school C-style iterations such as `for(int i=0; i<10; ++i)`
Orion Edwards
+3  A: 

Second form.

In my opinion, the less language constructs and keywords you have to use, the better. C# has enough extraneous crud in it as it is.

Generally the less you have to type, the better. Seriously, how could you not want to use "var" in situations like this? Surely if being explicit was your only goal, you'd still be using hungarian notation... you have an IDE that gives you type information whenever you hover over... or of course Ctrl+Q if you're using Resharper...

@T.E.D. The performance implications of a delegate invocation are a secondary concern. If you're doing this a thousand terms sure, run dot trace and see if it's not acceptable.

@Reed Copsey: re non-standard, if a developer can't work out what ".Each" is doing then you've got more problems, heh. Hacking the language to make it nicer is one of the great joys of programming.

mlangsworth
@mlangsworth: I have no idea what "Each" means. I'm not alone. ".Each" is obviously a custom extension method, either on IEnumerable<T>, IList<T>, or some concrete collection. I have no idea if it does any pre or post processing, or any other function other than calling the Action<T> it's receiving. I could guess, but I don't KNOW, from this code - with foreach, I KNOW, exactly, what is happening here.
Reed Copsey
@Reed Copsey: but every extension method is "custom", isn't it? Perhaps if the extension method was instead called, "ForEach"? But taken to the extreme, you never know what any library method does without looking at the source... So why assume anything other than the what the name and signature of said method tell you? If an extension method is of signature void ForEach<T>(this IEnumerable<T> list, Action<T> action), I reckon you bet safely that it just passes each element to the action.
mlangsworth
Sorry, should have put my custom extensions source in: .Each actually emails each person in your contact list a random chuck norris quote, .Where sets your wallpaper to a where's wally poster and .Select takes the result and sets it as your twitter status
Dave Newman
I'd agree with that. The only reason I brought it up is that I couldn't find a *primary* concern. :-)
T.E.D.
+2  A: 

The lamda version is actually not slower. I just did a quick test and the delegate version is about 30% faster.

Here is the codez:

class Blah {
    public void DoStuff() {
    }
}

        List<Blah> blahs = new List<Blah>();
        DateTime start = DateTime.Now;

        for(int i = 0; i < 30000000; i++) {
            blahs.Add(new Blah());
        }

        TimeSpan elapsed = (DateTime.Now - start);
        Console.WriteLine(string.Format(System.Globalization.CultureInfo.CurrentCulture, "Allocation - {0:00}:{1:00}:{2:00}.{3:000}",
         elapsed.Hours,
         elapsed.Minutes,
         elapsed.Seconds,
         elapsed.Milliseconds));

        start = DateTime.Now;

        foreach(var bl in blahs) {
            bl.DoStuff();
        }

        elapsed = (DateTime.Now - start);
        Console.WriteLine(string.Format(System.Globalization.CultureInfo.CurrentCulture, "foreach - {0:00}:{1:00}:{2:00}.{3:000}",
         elapsed.Hours,
         elapsed.Minutes,
         elapsed.Seconds,
         elapsed.Milliseconds));

        start = DateTime.Now;

        blahs.ForEach(bl=>bl.DoStuff());

        elapsed = (DateTime.Now - start);
        Console.WriteLine(string.Format(System.Globalization.CultureInfo.CurrentCulture, "lambda - {0:00}:{1:00}:{2:00}.{3:000}",
         elapsed.Hours,
         elapsed.Minutes,
         elapsed.Seconds,
         elapsed.Milliseconds));

OK, So I've run more tests and here are the results.

  1. The order of the execution(forach, lambda or lambda, foreach) didn't make much difference, lambda version was still faster:

    foreach - 00:00:00.561
    lambda - 00:00:00.389
    
    
    lambda - 00:00:00.317
    foreach - 00:00:00.337
  2. The difference in performance is a lot less for arrays of classes. Here are the numbers for Blah[30000000]:

    lambda - 00:00:00.317 
    foreach - 00:00:00.337
  3. Here is the same test but Blah being a struct:

    Blah[] version 
    lambda - 00:00:00.676 
    foreach - 00:00:00.437 
    
    
    List version:
    lambda - 00:00:00.461
    foreach - 00:00:00.391
  4. Optimized build, Blah is a struct using an array.

    lambda - 00:00:00.426
    foreach - 00:00:00.079

Conclusion: There is no blanket answer for performance of foreach vs lambda. The answer is It depends. Here is a more scientific test for List<T>. As far as I can tell it's pretty damn efficient. If you are really concerned with performance use for(int i... loop. For iterating over a collection of a thousand customer records (example) it really doesn't matter all that much.

As far as deciding between which version to use I would put potential performance hit for lambda version way at the bottom.

Conclusion #2 T[] (where T is a value type) foreach loop is about 5 times faster for this test in an optimized build. That's the only significant difference between a Debug and Release build. So there you go, for arrays of value types use foreach, everything else - it doesn't matter.

Igor Zevaka
That's interesting and unexpected. List<T>.Enumerator introduces more overhead than I'd have guessed, but note that it detects collection modification during the iteration while List<T>.ForEach does not (at least, in my local .NET assemblies).
Trillian
The biggest thing here is that you are giving an advantage to the lambda version by calling it second. The foreach loop will incur JIT overhead which is being included the measured time. Secondly (as Trillian mentioned) the List<T> enumerator is not particularly bare-metal. Using foreach on a T[] instead will give better performance still which is better than the lambda approach >2x faster on my machine. The raw performance may or may not be significant, but there is measurable cost to the extra indirection of the lambda approach. If Blah is a value type, this is very dramatic.
homeInAStar
This is true with List<T>.ForEach, but not in general. He explicitly had this listed as "Each", not List<T>'s ForEach method.
Reed Copsey
are you running an optimized build?
homeInAStar
Actually it's debug, i ll rerun the tests.
Igor Zevaka
+1 @Reed, I didn't even notice he had Each() instead of ForEach().
Jim Schubert
Which collections have `Each()` method? So far I can't find it on any native ones.
Igor Zevaka
It would be better if you would put those tests in subroutines and run them in such way --> test 1, test 2, test 1. The reason: caching.
macias
More tests: I tested lambda, foreach, for up (++index), for down (--index). For class type: the fastest is fordown, lambda, forup, foreach. For struct: foreach, fordown, lambda, forup. So for loop is not a winner in both cases.
macias
Cool, so for loop is not that flash, interesting...
Igor Zevaka
+6  A: 

Here you're doing some very imperative things like writing a statement rather than an expression (as presumably the Each method returns no value) and mutating state (which one can only assume the methods do, as they also appear to return no value) yet you're trying to pass them off as 'functional programming' by passing a collection of statements as a delegate. This code could barely be further from the ideals and idioms of functional programming, so why try to disguise it as such?

As much as I like multi-paradigm languages such as C#, I think they are easiest to understand and maintain when paradigms are mixed at a higher level (e.g. an entire method written in either a functional or an imperative style) rather than when multiple paradigms are mixed within a single statement or expression.

If you're writing imperative code just be honest about it and use a loop. It's nothing to be ashamed of. Imperative code is not an inherently bad thing.

Greg Beech
+1  A: 

I think extension methods are cool, but I think break and edit-and-continue are cooler.

Robert Rossney
Do extension methods somehow affect edit-and-continue? I don't get it :-S
Orion Edwards
Extension methods don't, but anonymous methods do.
Robert Rossney