Using IEnumerable nonsense for everything
-
That's a matter of opinion, in one of the articles posted on this thread MS explain why there is no ForEach for List and the reason being that it is less readable and offers no real advantage over the native foreach. You also forget "harder to debug" :)
As others have pointed out,
ForEachis the odd man out here. Aforeachloop of the results of the sequence returned from LINQ is the better option. But the bulk of LINQ is about telling the compiler what to do, not how to do it. And that makes the code much more readable (for some). Imagine you start with a big block of code in a single method. It takes a list, filters it, sorts it, groups it, filters it some more, projects it, and then processes it. You've got a fairly complex method which is specific to one task. If you need to repeat any of those operations, you have to duplicate the code. The first thing you would do is refactor the code, to move some of the common operations out into separate, simpler, reusable methods. You could then write simple unit tests for those methods, without having to set up the more complicated data for your original method, and without having to work out which part of the original method failed if the tests failed. Then, you would reuse those simpler methods elsewhere when you needed to do the same thing. Need to filter a list? CallSomeClass.FilterAList. Need to group a list? CallSomeClass.MakeSomeGroups. Pretty soon, you end up with a collection of utility methods that you're reusing everywhere. But the syntax is quite nasty:var source = GetAList();
var filtered = SomeClass.FilterAList(source, SomeFilter);
var sorted = SomeClass.MakeItSorted(filtered, SomeSortingCondition);
var grouped = SomeClass.MakeSomeGroups(sorted, SomeGroupingCondition);
var filteredAgain = SomeClass.FilterAList(grouped, AnotherFilter);
var result = SomeClass.ProjectAList(filteredAgain, SomeProjection);// Or:
var result = SomeClass.ProjectAList(
SomeClass.FilterAList(
SomeClass.MakeSomeGroups(
SomeClass.MakeItSorted(
SomeClass.FilterAList(
GetAList(),
SomeFilter),
SomeSortingCondition),
SomeGroupingCondition),
AnotherFilter),
SomeProjection);To tidy it up, you would like to be able to call each utility method as if it was defined on the
IEnumerable<T>interface. You can't add the methods to the interface, since that would break everything that implemented it. So instead, you introduce extension methods, and the syntax becomes:var result = GetAList()
.FilterAList(SomeFilter)
.MakeItSorted(SomeSortingCond -
BillWoodruff wrote:
I get a glimpse of your shadow going around a corner
You are generous as always! There are some corners I probably should not be followed:
public static bool If(this bool b, Action action)
public static void IfElse(this bool b, Action ifTrue, Action ifFalse)
etc. Let's just call those "experiments." :) Marc
Imperative to Functional Programming Succinctly Contributors Wanted for Higher Order Programming Project! Learning to code with python is like learning to swim with those little arm floaties. It gives you undeserved confidence and will eventually drown you. - DangerBunny
Ha! But then there is value in experiments:
public static void ForEach<T>(this IEnumerable<T> sequence, Action<T, int> action)
{
// argument null checking omitted
var index = 0;
foreach(T item in sequence) {
action(item, index);
index++;
}
}For example, I am curious if I could find value in (probably with a different body implementation):
public static async Task ForEachAsync<T>(this IEnumerable<T> sequence, Func<T, Task> actionAsync)
{
// argument null checking omitted
foreach(T item in sequence) await actionAsync(item);
} -
Uhm, I thought you said any use of ToArray is a cry for help?
Regards, Nish
Website: www.voidnish.com Blog: voidnish.wordpress.com
You'll note that Harold original code line
someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e));
won't actually work. To get it to compile, you'd need to change it to :
someStuff.Where(c => c != What).Select(d => d + The).ToList().Foreach(e => Hell(e));
which is I think, the type of thing PIEBALDconsult was referring to.
Truth, James
-
You'll note that Harold original code line
someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e));
won't actually work. To get it to compile, you'd need to change it to :
someStuff.Where(c => c != What).Select(d => d + The).ToList().Foreach(e => Hell(e));
which is I think, the type of thing PIEBALDconsult was referring to.
Truth, James
Unless they had an extension method that implemented
ForEachonIEnumerable<T>.Regards, Nish
Website: www.voidnish.com Blog: voidnish.wordpress.com
-
Writing all of your code in one big
Mainfunction is faster than any of this "object-oriented" nonsense. And using C or assembly will be much faster than this JIT-compiled C# nonsense. Of course, it will take a lot longer to write, and be much harder to debug. But premature optimization is much more important than sleep! ;P
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
Richard Deeming wrote:
Writing all of your code in one big
Mainfunction is faster than any of this "object-oriented" nonsense.That is, almost certainly, not true. With one big function, the optimizer will have to practically shut-down. Many smaller functions can be highly optimized.
Richard Deeming wrote:
And using C or assembly will be much faster than this JIT-compiled C# nonsense.
Again, real world examples have shown that letting the computer do things like managing your resources, is much faster than trying to do it yourself manually.
Truth, James
-
You've probably seen this style if you're done anything with C# after 2007 or so. someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e)); Instead of, you know, a plain old `for` loop with an `if` in it and so on. Or maybe `foreach` if you want to be fancy. So, now we have nearly a decade of experience with this, can we finally settle this question: Is this style cancer? I still think it is, and the retort "you just have to get used to it" isn't going to work any more. I file this firmly under "stupid one-liner 'clever' code with no benefits to compensate". Yes, I've argued in the past that "clever code" isn't necessarily bad, and I'll keep saying that - there's a time and a place for it. But not if you're just trying to be cute. "Oh look at me, I put everything on one line, +1 nerd points for me" And this is even worse. It's not just cute with no benefits to compensate, it's cute and harder to read. Side question, why is this style popular?
We *should* be using that syntax because it's cleaner and more readable than lots of indented loops. Our problems (at this point) are 1. It's inefficient 2. There's not enough good guidance on not doing dumb things (ToArray etc being case in point). EF does a helluva job converting LINQ to SQL so what would be interesting is if there was a preprocesser that went through the whole LINQ chain, worked out what was really happening, then optimised (ie not did a bunch of stuff, parallelised other stuff, vectorised some stuff etc) and made it more efficient than foreach loops (which themselves are not efficient). We shouldn't have a million devs optimising the same code. We should be able to express the code in elegant syntax and have the tools do the optimisation.
cheers Chris Maunder
-
As others have pointed out,
ForEachis the odd man out here. Aforeachloop of the results of the sequence returned from LINQ is the better option. But the bulk of LINQ is about telling the compiler what to do, not how to do it. And that makes the code much more readable (for some). Imagine you start with a big block of code in a single method. It takes a list, filters it, sorts it, groups it, filters it some more, projects it, and then processes it. You've got a fairly complex method which is specific to one task. If you need to repeat any of those operations, you have to duplicate the code. The first thing you would do is refactor the code, to move some of the common operations out into separate, simpler, reusable methods. You could then write simple unit tests for those methods, without having to set up the more complicated data for your original method, and without having to work out which part of the original method failed if the tests failed. Then, you would reuse those simpler methods elsewhere when you needed to do the same thing. Need to filter a list? CallSomeClass.FilterAList. Need to group a list? CallSomeClass.MakeSomeGroups. Pretty soon, you end up with a collection of utility methods that you're reusing everywhere. But the syntax is quite nasty:var source = GetAList();
var filtered = SomeClass.FilterAList(source, SomeFilter);
var sorted = SomeClass.MakeItSorted(filtered, SomeSortingCondition);
var grouped = SomeClass.MakeSomeGroups(sorted, SomeGroupingCondition);
var filteredAgain = SomeClass.FilterAList(grouped, AnotherFilter);
var result = SomeClass.ProjectAList(filteredAgain, SomeProjection);// Or:
var result = SomeClass.ProjectAList(
SomeClass.FilterAList(
SomeClass.MakeSomeGroups(
SomeClass.MakeItSorted(
SomeClass.FilterAList(
GetAList(),
SomeFilter),
SomeSortingCondition),
SomeGroupingCondition),
AnotherFilter),
SomeProjection);To tidy it up, you would like to be able to call each utility method as if it was defined on the
IEnumerable<T>interface. You can't add the methods to the interface, since that would break everything that implemented it. So instead, you introduce extension methods, and the syntax becomes:var result = GetAList()
.FilterAList(SomeFilter)
.MakeItSorted(SomeSortingCondI'm talking about using linq to foreach a collection vs using foreach. As I said in my post, it is fine to use linq if you are getting advantages such as in the example you just posted, but I thought I made it pretty clear that was not the kind of code I was talking about and also that I never said to never use linq.
Richard Deeming wrote:
If you stick to debugging your own code, it's easier to debug, because there's less of it
var result = GetAList()
.FilterAList(SomeFilter)
.MakeItSorted(SomeSortingCondition)
.MakeSomeGroups(SomeGroupingCondition)
.FilterAList(AnotherFilter)
.ProjectAList(SomeProjection);That line throws a null exception...can you look at the line that threw the exception and know what the issue is?
-
I'm talking about using linq to foreach a collection vs using foreach. As I said in my post, it is fine to use linq if you are getting advantages such as in the example you just posted, but I thought I made it pretty clear that was not the kind of code I was talking about and also that I never said to never use linq.
Richard Deeming wrote:
If you stick to debugging your own code, it's easier to debug, because there's less of it
var result = GetAList()
.FilterAList(SomeFilter)
.MakeItSorted(SomeSortingCondition)
.MakeSomeGroups(SomeGroupingCondition)
.FilterAList(AnotherFilter)
.ProjectAList(SomeProjection);That line throws a null exception...can you look at the line that threw the exception and know what the issue is?
F-ES Sitecore wrote:
That line throws a null exception...can you look at the line that threw the exception and know what the issue is?
Assuming I'm using LINQ, the most likely culprit would be
GetAListreturningnull. Failing that, I'd have a stack-trace to tell me where the exception occurred. :)
"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer
-
You've probably seen this style if you're done anything with C# after 2007 or so. someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e)); Instead of, you know, a plain old `for` loop with an `if` in it and so on. Or maybe `foreach` if you want to be fancy. So, now we have nearly a decade of experience with this, can we finally settle this question: Is this style cancer? I still think it is, and the retort "you just have to get used to it" isn't going to work any more. I file this firmly under "stupid one-liner 'clever' code with no benefits to compensate". Yes, I've argued in the past that "clever code" isn't necessarily bad, and I'll keep saying that - there's a time and a place for it. But not if you're just trying to be cute. "Oh look at me, I put everything on one line, +1 nerd points for me" And this is even worse. It's not just cute with no benefits to compensate, it's cute and harder to read. Side question, why is this style popular?
Plain-old-for definitely will work too, but this "one-liner" is specially intended for simple cases, where "for" is just too much! Say, you need just checked checkboxes:
var chs = AllChBoxes.Where(box => box.IsCheched);// DONE!
...and now look what you have to do with for:
var chs = new List();// - note, you have to create materialized list, not just Enumerable!
foreach(var ch in AllChBoxes)
if (ch.IsChecked) chs.Add(ch);You write THREE lines (what is obviously more to read + more error prone) and achieved... even worse result, since IEnumerable in one-liner takes less memory (if needed at all). So get your a$$ from the criocamera and study new way! :)
-
Plain-old-for definitely will work too, but this "one-liner" is specially intended for simple cases, where "for" is just too much! Say, you need just checked checkboxes:
var chs = AllChBoxes.Where(box => box.IsCheched);// DONE!
...and now look what you have to do with for:
var chs = new List();// - note, you have to create materialized list, not just Enumerable!
foreach(var ch in AllChBoxes)
if (ch.IsChecked) chs.Add(ch);You write THREE lines (what is obviously more to read + more error prone) and achieved... even worse result, since IEnumerable in one-liner takes less memory (if needed at all). So get your a$$ from the criocamera and study new way! :)
Here it's still obvious. It's the chaining where things start to get confusing. Besides, I can't agree with your statement that you must create a list, after all you need those checkboxes in order to *do something* with them, you can most of the time just do that in the very same loop that checks them.
-
You've probably seen this style if you're done anything with C# after 2007 or so. someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e)); Instead of, you know, a plain old `for` loop with an `if` in it and so on. Or maybe `foreach` if you want to be fancy. So, now we have nearly a decade of experience with this, can we finally settle this question: Is this style cancer? I still think it is, and the retort "you just have to get used to it" isn't going to work any more. I file this firmly under "stupid one-liner 'clever' code with no benefits to compensate". Yes, I've argued in the past that "clever code" isn't necessarily bad, and I'll keep saying that - there's a time and a place for it. But not if you're just trying to be cute. "Oh look at me, I put everything on one line, +1 nerd points for me" And this is even worse. It's not just cute with no benefits to compensate, it's cute and harder to read. Side question, why is this style popular?
Agreed, this is cancer. IEnumerable and Linq will cause far more instructions to be processed by the processor than a simple "for" loop with an "if". Will the average user notice this? No, not on todays computers. But as more people do these trick things, it does build up. I remember when trick things were done to save processor clock ticks, and it was just as bad for reading code. The smart ones would document the clever code with comments with a reasoning why it had to be done. if you've seen that in production code before, people are trying to be clever for clever sake, I just don't see the benefit.
-
You've probably seen this style if you're done anything with C# after 2007 or so. someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e)); Instead of, you know, a plain old `for` loop with an `if` in it and so on. Or maybe `foreach` if you want to be fancy. So, now we have nearly a decade of experience with this, can we finally settle this question: Is this style cancer? I still think it is, and the retort "you just have to get used to it" isn't going to work any more. I file this firmly under "stupid one-liner 'clever' code with no benefits to compensate". Yes, I've argued in the past that "clever code" isn't necessarily bad, and I'll keep saying that - there's a time and a place for it. But not if you're just trying to be cute. "Oh look at me, I put everything on one line, +1 nerd points for me" And this is even worse. It's not just cute with no benefits to compensate, it's cute and harder to read. Side question, why is this style popular?
-
You've probably seen this style if you're done anything with C# after 2007 or so. someStuff.Where(c => c != What).Select(d => d + The).Foreach(e => Hell(e)); Instead of, you know, a plain old `for` loop with an `if` in it and so on. Or maybe `foreach` if you want to be fancy. So, now we have nearly a decade of experience with this, can we finally settle this question: Is this style cancer? I still think it is, and the retort "you just have to get used to it" isn't going to work any more. I file this firmly under "stupid one-liner 'clever' code with no benefits to compensate". Yes, I've argued in the past that "clever code" isn't necessarily bad, and I'll keep saying that - there's a time and a place for it. But not if you're just trying to be cute. "Oh look at me, I put everything on one line, +1 nerd points for me" And this is even worse. It's not just cute with no benefits to compensate, it's cute and harder to read. Side question, why is this style popular?
Yeah - I hate that sort of shit as well. Write-only programs. I think people think they're clever, or something: "Those of you who think you're intelligent are annoying to those of us who are"(tm)...
-
Here it's still obvious. It's the chaining where things start to get confusing. Besides, I can't agree with your statement that you must create a list, after all you need those checkboxes in order to *do something* with them, you can most of the time just do that in the very same loop that checks them.
You said "for vs LINQ", I show you obvious case where you're not right. If you wanna just discuss how long LINQ can be - it's different question. You don't understand word "materialized". If you use "for", you have to create physical list, where you keep your objects. In case of LINQ you have Enumerator, which will not take any object until you ask! It's important difference when you have billion objects, where half of 'em match your query. Enumerator just pass 'em one-by-one (keeping memory consumption low exactly for ONE ELEMENT), while your "for" takes all necessary memory at once.
-
You said "for vs LINQ", I show you obvious case where you're not right. If you wanna just discuss how long LINQ can be - it's different question. You don't understand word "materialized". If you use "for", you have to create physical list, where you keep your objects. In case of LINQ you have Enumerator, which will not take any object until you ask! It's important difference when you have billion objects, where half of 'em match your query. Enumerator just pass 'em one-by-one (keeping memory consumption low exactly for ONE ELEMENT), while your "for" takes all necessary memory at once.
Thornik wrote:
You said "for vs LINQ",
I did not, I showed an example of what I deem unreasonable. Using a single clause is still clear, if often unnecessary, though I like Max for example - there's a case where it really is simpler than an equivalent explicit loop.
Thornik wrote:
You don't understand word "materialized". If you use "for", you have to create physical list, where you keep your objects.
Oh you mean the source, sure. `foreach` it is then, problem solved.
-
Thornik wrote:
You said "for vs LINQ",
I did not, I showed an example of what I deem unreasonable. Using a single clause is still clear, if often unnecessary, though I like Max for example - there's a case where it really is simpler than an equivalent explicit loop.
Thornik wrote:
You don't understand word "materialized". If you use "for", you have to create physical list, where you keep your objects.
Oh you mean the source, sure. `foreach` it is then, problem solved.
You shown long LINQ query and start talking about "foreach". But even being that long, FORMATTING RULES. :)
someStuff.Where(c => c != What) // comment why you do it
.Select(d => d + The) // comment why you do it
.Foreach(e => Hell(e));// comment why you do it> Oh you mean the source, sure. foreach it is then, problem solved. Nope. You do not solve problem if you prepare list of objects thru foreach - you have to put 'em in a List<> (because hell knows how it will be used later). In case of LINQ you prepare just REQUEST (which takes zero memory), which later will enumerate any amount of objects. And BTW same request can be enumerated many times.
-
You shown long LINQ query and start talking about "foreach". But even being that long, FORMATTING RULES. :)
someStuff.Where(c => c != What) // comment why you do it
.Select(d => d + The) // comment why you do it
.Foreach(e => Hell(e));// comment why you do it> Oh you mean the source, sure. foreach it is then, problem solved. Nope. You do not solve problem if you prepare list of objects thru foreach - you have to put 'em in a List<> (because hell knows how it will be used later). In case of LINQ you prepare just REQUEST (which takes zero memory), which later will enumerate any amount of objects. And BTW same request can be enumerated many times.
-
No, he meant to execute. I set up some experimental code that looped many times using the various methods like native code and linq and timed them. I also pointed out that the linq code was using anonymous methods and that they had overhead too.
Well I do find it a lot faster to write. I would expect it to have poorer performance, but don't know how bad. I believe is that if you have performance issues, find out the methods that use the most resources, and optimize them. This is where get most bang for the buck.
-
harold aptroot wrote:
Is this style cancer?
Yes. Many fans of that style don't realize how many times the data gets copied and iterated when they do nonsense like that. What really irks me is the near-constant use of
ToListorToArray; those are definitely cries for help. Even a simpleforeachshould generally be avoided in situations where aforwill perform at least as well.I would argue the total opposite. This is not cancer. The data does not get copied and iterated because this is all lazy processed. Perhaps you dont understand how these methods actually work. Do you know how many times it took me hours to construct a proper nested for loop? With this style the complex can be accomplished in minutes. You need to call ToList when you are complete to finally iterate over the entire IEnumerable. Because the Where and Select methods are lazy processed I have found that you dont get the proper results at the end unless you call ToList. Also calling ToList is needed when performing async or multi threaded code. You need your own copy of the items to mess with otherwise you will get errors. As for performance we are talking small milliseconds longer. I can read and understand that one liner perfectly in 5 seconds. Can you honestly say that a for loop is perfectly understandable in that amount of time? I think not. Also did you know you can iterate over thousands of records in the same amount of time it takes to do an if(x == y) statement. "if" comparisons are the slowest code to run.
-
Richard Deeming wrote:
Writing all of your code in one big
Mainfunction is faster than any of this "object-oriented" nonsense.That is, almost certainly, not true. With one big function, the optimizer will have to practically shut-down. Many smaller functions can be highly optimized.
Richard Deeming wrote:
And using C or assembly will be much faster than this JIT-compiled C# nonsense.
Again, real world examples have shown that letting the computer do things like managing your resources, is much faster than trying to do it yourself manually.
Truth, James
I would be a bit surprised if your first point was true. Please give me a couple examples of optimizations that depend on function size.
