2

I was playing around with the main differences between 4 monitor classes/approaches, like async/await, Task, BackgroundWorker and Thread), so I created some controls, which just interact with those 4 approaches/objects. I also wanted to dig into LINQ/LAMBDA at the same time and managed to write some successful LINQ-statements like this one, for example.

                 (from t in tabControlOutput.TabPages.Cast<TabPage>()
                  from c in t.Controls.OfType<WebBrowser>()
                  select c).ToList().ForEach(c => c.Navigate(Constants.BlankUrl));

Of course I was looking before posting this question and I found some nice information like this

First Link

Which redirects to a comment of Eric Lippert.

foreach vs ForEach

Besides that I am aware of LINQ delivering some overhead to standard-operations, which can be issued by standard instructions instead of linq, the overhead issue seems to be the particular reason fore MS, why stick to standard LINQ-less processing, but E.Lippert has some other arguments, especially referring to ForEach.

There are some statements, which seem to confuse me:

.... The purpose of an expression is to compute a value, not to cause a side effect. The purpose of a statement is to cause a side effect.

I never heard / read about that in any programming book, best practice or whatever. Following common (and my) experience, the term sideeffects usually carries a negative taste of arbitrary , instead of being an aim, which is achieved willingly.

Could anyone clarifiy this quote from E.Lippert ?

Furthermore E.Lippert states, that some two lines seem to be harder to maintain, which I would not accept(ok, this might be opinion based).

But, regarding my piece of code, I can see that the only "ugly" thing is the "Cast".

So, would there be a reasonable reason(not opinion based, but in pure technical terms, arguments, restrictive statements, principles (readability also belongs to them), why my lines should or should not be replaced by traditional foreach, or even reflection ?

EDIT: I changed my code, remmed out my previous lines, made comments and added those lines. Please feel free to comment on them:

// ---> NOT RECOMMENDED APPROACH: Because LINQ is more designed to query, but at the end
                //                                There is a modification of the queried objects, which
                //                                Is not real LINQ, but a mix. 
                //                                And it violates the principles of least astonishment/surprise

                // Changed to
                var qresult = (from t in tabControlOutput.TabPages.Cast<TabPage>()
                               from c in t.Controls.OfType<WebBrowser>()
                               select c);
                foreach (var tmp in qresult)
                {
                    tmp.Navigate(Constants.BlankUrl);
                }
Community
  • 1
  • 1
icbytes
  • 1,831
  • 1
  • 17
  • 27
  • 1
    No, it's definitely going to be opinion-based in general. In your specific case, your code is unnecessarily inefficient as you're calling `ToList` which could be avoided if you just iterated over the result, but there's the opinion-based argument for cases where you *already* have a `List`. – Jon Skeet Jul 06 '16 at 08:48
  • Even, if we stick to pure technical terms/arguments as much as possible ? – icbytes Jul 06 '16 at 08:49
  • By *not to cause a side effect* it is meant that (it is expected) no state should not be mutated. Your LINQ expression, up to the point where you called `.ToList()` should be (is supposed to be) callable multiple times without changing the outcome, like a pure function. LINQ is also lazy, so an expression `list.Where(i => i > 5)` actually does nothing by itself, until you evaluate it. So the `ForEach` extension definitely has different behavior. – vgru Jul 06 '16 at 08:51
  • Ok, so for me this is already a nice argument. Why should I get that result into a "var query", which will return a List, when I can save that extra var ? Who would do such things at all and why ? So, Jon, You say, inefficient... in terms of performance, because I use the list implicitely instead of iterating aafterwards.. Do You mean, this makes it less performant ? – icbytes Jul 06 '16 at 08:51
  • @icbytes: if using a plain `for each` loop, there doesn't have to be a list at all. You just iterate through the lazy enumerator. A "result" of the LINQ expression is an expression tree, not a list, so the `query` variable would simply have a reference to the expression tree. – vgru Jul 06 '16 at 08:53
  • @Groo: This I know. ( But, not the word expression tree, I must admit, just reading about it ). – icbytes Jul 06 '16 at 08:54
  • It's a bad reason trying to post-facto justify an odd decision to have a `ForEach` operator on `IList` but not `IEnumerable`. It's especially odd now you can easily debug LINQ expressions, which was one of his major objections. However, that's my opinion, and this question is too opinion-based and should be closed. – mattmanser Jul 06 '16 at 08:56
  • @mattmaner: But I am sure, You can give reasons for that, which are not opinion based, but simply follow design principles or even technical reasons. If You still vote to close, please feel free to do so. – icbytes Jul 06 '16 at 08:59
  • "Could anyone clarifiy this quote from E.Lippert ?" I don't understand what you're asking for clarification on. If your question is "did Eric mean to imply that "side effects" are unwanted effects?" then no, I did not. I probably should have simply said "effects", to be more clear. – Eric Lippert Jul 07 '16 at 15:56
  • Exactly that was part of my question. – icbytes Jul 07 '16 at 15:58

1 Answers1

2

First, let me get a bit nitpicky about what LINQ is.

This is LINQ. Language integrated query. You are querying. Reading.

(from t in tabControlOutput.TabPages.Cast<TabPage>()
 from c in t.Controls.OfType<WebBrowser>()
 select c).ToList()

The following is not. Technically, it's not LINQ (a set of extension methods) but a normal class member of the List<T> class. And philosophically, this is not querying. You are manipulating data:

.ForEach(c => c.Navigate(Constants.BlankUrl));

Your call to .ToList() is only needed so you can call the .ForEach method of said class. It would be more efficient if you just used a normal foreach on the result of your LINQ.

So your line is not plain LINQ from the start, it's actually manipulating data, which may come as a surprise to the casual reader thinking that it was plain LINQ and therefore violates the principle of least surprise. And last but not least it's less efficient than the foreach alternative.

nvoigt
  • 75,013
  • 26
  • 93
  • 142