When does lambda in an extension method do too much?

Question

I understand that this is partially subjective, but I'm generally interested in learning from the community and have not been able to successfully find an existing issue that solves this problem.

I am participating in several religious debates with a colleague over a particular Select statement in an L2EF request.

.Select(r => { r.foo.Bar = r.bar; r.foo.Bar.BarType = r.Alpha; if (r.barAddress != null) { r.foo.Bar.Address = r.barAddress; r.foo.Bar.Address.State = r.BarState; } if (r.baz != null) { r.foo.Bar.Baz = r.baz; if (r.bazAddress != null) { r.foo.Bar.Baz.Address = r.bazAddress; r.foo.Bar.Baz.Address.State = r.BazState; } } return r.foo; }) 

Cautions:

• This is Linq-to-Entities
• This is after working in the database, as was done and returned
• The input parameter r is anonymous

Personally, I believe that (a) the select clause should not change the values, it should just be projected. His counter argument is that he does not change anything, he just made sure that everything was correctly initialized as a result of the database query. Secondly, I think that as soon as it starts filling out the complete blocks of code and returns instructions, it's time to define a method or even Func<T, U> , and not do it all inline. In addition, the complicating element is again anonymous, so you must determine the type. Nevertheless, we are still discussing a general question, if not a specific one.

So, when does a lambda expression do too much? Where do you draw a fuzzy line in the sand?

Answer 1

My first instinct is to agree with you, especially on the issue of size and complexity.

However, it is used in a context where it will be (or sometimes executed) executed as something other than .NET code (especially if it is turned into part of an SQL query), I will become much more tolerant of it.

So this is where I draw the fuzzy line, and also why I move it again :)

Answer 2

I also agree that Select () should be used for design. I would prefer to use the "let" keyword for preliminary validation so that this projection in Select () can be clean. This will allow Select () to refer to the variables (s) specified with "let".

Answer 3

I don't think this is a long lambda expression, personally. I think you will see much more complex and nested lambdas in the future. Especially with something like Rx.

As for state changes ... well, here it just initializes the values. It would only bother me if he assigned the state some variable outside of the lambda, but this is all initialization r, so it seems fine to me.

Answer 4

I had to look at this for a while before I saw what annoyed me.

• This requires refactoring.

• The fact that it took me so long to read the intent of lambda.

I'm not sure if I can take part in defining the Select assignment, but I agree that the shorter you can save the lambda, the better. Break it down for reuse if it is necessary that the initialization after sampling the dB is so bad.

Answer 5

You have to consider other alternatives ... is there a better place to put this logic. In any case, you iterate over each object in LINQ or in the foreach loop to do this extra work ... If you don't want to reorganize it into your own extension method, I would say leave it if it works.

Visually, it's not so dirty, so that's a plus. Or you can find out if the let keyword (basically a subquery) is buying something else.

Answer 6

The length of the lambda doesn't bother me at all. But I agree with the feeling of dirt when you assign a value in your select statement. I would include the initialization stuff in the statement just below the select statement.