Using IEnumerable nonsense for everything
-
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.
-
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.
Optimization largely depends on tracking the lifetime of variables:
for(int i =0; i< 100; ++i)
{...}will be better optimized, than this...
int i;
for(i =0; i< 100; ++i)
{...}just because the compiler knows that "i" is never used again outside that for loop. In the latter, space must be allocated for i on the stack, and it must be stored there. In the first, "i" may live at it's entire existence in a register. Now, in an example as small as the above, a good compiler may still realize that even the second "i" is not used again, but the larger that function gets, with more things to track, the optimizer begin to give up.
Truth, James
-
Optimization largely depends on tracking the lifetime of variables:
for(int i =0; i< 100; ++i)
{...}will be better optimized, than this...
int i;
for(i =0; i< 100; ++i)
{...}just because the compiler knows that "i" is never used again outside that for loop. In the latter, space must be allocated for i on the stack, and it must be stored there. In the first, "i" may live at it's entire existence in a register. Now, in an example as small as the above, a good compiler may still realize that even the second "i" is not used again, but the larger that function gets, with more things to track, the optimizer begin to give up.
Truth, James
That might be true but it comes at the cost of jumps and stack management for function calls. I think you will be hard-pressed to find an example of one block of code that runs slower than similar code split into more functions.
-
For debugging, though, if you convert the lambda expression into a lambda statement, you can put a breakpoint in it. That can help in debugging. Usually what I end up doing is split the fluent chain into separate pieces if I need to debug it. That being said, this is usually a style I employ as I'm finishing up code and have already tested it while doing my normal refactoring. (and when I know high performance is not necessary)
I just set breakpoints on the lambda expressions in the statements. The debugger is a little finicky on setting them there -- you have to click somewhere in the RHS of the lambda and then hit F9 to set the breakpoint. No harder to debug than any other bit of code. I use Linq a lot, but for non-performant areas. I don't expect it to be faster to run, only faster to write, debug and maintain.
We can program with only 1's, but if all you've got are zeros, you've got nothing.
-
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.
-
irneb wrote:
your sample is quite litterally performing 3 loops where one for loop would have sufficed.
you should really take a look on what the compiler does when it enconters the yield keyword, you might be surprised to find out it's not as inefficient as you think.
Sentenryu wrote:
you should really take a look on what the compiler does when it enconters the yield keyword
Not even slightly sure what you mean by that ??? Here's a test: BenchLinqLoops[^] The chain of Linq statements clearly perform 3 loops (one of which is unnecessary). Yield doesn't "magically" fix that. And to show by how much such Linq chains (and even the Linq-to-SQL version) adds extra overhead - look at the performance of that yield function.
-
irneb wrote:
your sample is quite litterally performing 3 loops where one for loop would have sufficed.
you should really take a look on what the compiler does when it enconters the yield keyword, you might be surprised to find out it's not as inefficient as you think.
And just to make doubly sure no JIT optimizations skew the benchmark - I added an extra loop before starting the stopwatch, and rearranged the orders: BenchLinqLoops[^] Now you actually see the SQL variant's "extra" overhead - i.e. being translated into (effectively) the LinqAddXToEven2Loops function before it runs.
-
I promise you I'm not just flaming, I just really find those chained LINQy things hard to read. It's not like I haven't tried, I've been reading them for nearly a decade, I'm not getting used to them. It takes an extra step in my mind somehow, normal code I read and build up a picture of it in my mind, LINQy stuff I read, tear down, then build a picture. Clearly that is not the case for everyone here
I can certainly understand, especially when you are dealing with SelectMany, or Join. Those can get nasty, but then the code to do them would be nasty anyway. so I do not see a good reason to code it with foreach and if statements.
