|
|
You're so NICE! Very commendable. 
Marc
|
|
|
|
|
There's no reason not to be, just a honest mistake. 
|
|
|
|
|
Good to see polite folks on tech forums, getting to be a rarity these days
|
|
|
|
|
Thanks
|
|
|
|
|
Yeah, no kidding
|
|
|
|
|
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?
|
|
|
|
|
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 ToList or ToArray; those are definitely cries for help.
Even a simple foreach should generally be avoided in situations where a for will perform at least as well.
|
|
|
|
|
PIEBALDconsult wrote:
What really irks me is the near-constant use of ToList or ToArray; those are definitely cries for help.
Depends, if you are getting it off EF or OData, sometimes you want to do in-memory processing. Specially within services.
|
|
|
|
|
Uh huh, so why can't you?
|
|
|
|
|
Uhm, I thought you said any use of ToArray is a cry for help?
|
|
|
|
|
|
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 ForEach on IEnumerable<T>.
|
|
|
|
|
PIEBALDconsult wrote: Many fans of that style don't realize how many times the data gets copied and iterated when they do nonsense like that.
Many fans do realize, we just don't care 
Would I use the style for loops that should be executed milion times a second, like image processing? No.
Would I use it for everything else? Hell yes.
|
|
|
|
|
Then you're not the problem.
|
|
|
|
|
Why? A for loop tends to be far clearer and easier to read, not to mention faster and more efficient.
|
|
|
|
|
obermd wrote: A for loop tends to be far clearer and easier to read
Because that's your opinion and when I code I don't take your opinions into account
|
|
|
|
|
PIEBALDconsult wrote: how many times the data gets copied and iterated
Iterated: yes.
Copied: only if you use ToList / ToArray / ToDictionary / etc.
That's the big benefit of lazy evaluation - nothing gets iterated or copied until you're ready to use it.
Of course, if you don't understand what you're doing, that can also the big drawback. 
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Exactly. The point being that many do not realize what they're doing.
|
|
|
|
|
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.
|
|
|
|
|
For reference, a branch misprediction is less than 20 cycles on any reasonable µarch (ie excluding NetBurst), you can't do thousands of anything in that time.
|
|
|
|
|
Another issue is trying to debug it. If you get an error or exception then it's harder to debug when it's effectively a single statement.
harold aptroot wrote: Side question, why is this style popular?
I worked with someone that used linq wherever possible. His argument was that it was "faster". I think people think that because something is new it's fast *shrug*
|
|
|
|
|
That's really weird, I would have thought that anyone who likes speed tests to see how fast it really is, or at least looks up someone elses test.. and then they'd never use this style again.
|
|
|
|
|
Or you know, some one can be aware of the costs and still make decision to use this style? Almost as if speed is not top priority all the time. In parts that you really care about performance - write it in C++, slap managed wrapper around and call it a day. For everything else - enjoy modern1 features which make your life easier.
1 - if you can call something that is 10 years modern, in programming world.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
