Favour Verbose Linq expressions?

Question

It occurred to me that I write out linq statements in a simple, but what others may define as verbose manner;

A simple example:

return _entries
    .Where(x => x.Context.Equals(context))
    .Where(x => x.Type == typeof (T))
    .Select(x=>x.Value)
    .Cast<T>()
    .Single();

can be simplified to:

return _entries
    .Where(x => x.Context.Equals(context) && x.Type == typeof (T))
    .Select(x=>(T)x.Value)
    .Single();

[Question] In the long run, which is the better coding practice? i.e. long (and simple) linq chains or short linq chains with more complicated selectors/etc?

It is right to assume that these Linq statements will be optimized by the compiler?

@Lee: it's not. However, it _is_ shorter than `Select(x => x.Value).Cast()` — Nuffin, Mar 01 '13 at 17:05
Your type check is suspicious. Suppose x.Type is Giraffe and T is Animal. The cast will succeed, but the item is rejected by the Where clause because Giraffe and Animal are not the same type. I suspect that you want the `OfType` helper method. — Eric Lippert, Mar 01 '13 at 17:52

score 9 · Accepted Answer · edited Jan 01 '19 at 19:59

In the long run, which is the better coding practice?

I prefer short and simple. It's more readable. The whole point of LINQ is to make the code read more like the logic of the business domain.

It is right to assume that these Linq statements will be optimized by the compiler?

No; the optimization is done by the runtime, not by the compiler. LINQ-to-objects "Where" and "Select" clauses that follow the pattern you describe are optimized into a single "where-select" object at runtime to avoid creating too many iterators. (Though as Jon Skeet discovered, that can sometimes produce situations in which performance is actually degraded; like almost all "optimizations", it's not a win 100% of the time. Unfortunately I can't find Jon's article on that at this moment.)

Eric, I am guessing this is the article you were looking for: http://msmvps.com/blogs/jon_skeet/archive/2011/06/16/linq-to-objects-and-the-performance-of-nested-quot-where-quot-calls.aspx — SolutionYogi, Mar 01 '13 at 21:24

Daniel A.A. Pelsmaeker · Answer 2 · 2013-03-04T09:53:49.453

5

No, it is not right to assume these LINQ statements are optimized by the compiler. The more verbose syntax produces more Enumerator objects, so the two are not equivalent. From a performance standpoint the shorter syntax will be (slightly) better.

However, if you are choosing between the two syntaxes from a readability and coding standpoint, I would go with the first as it shows the steps that are taken better. I always prefer readability above performance, except when you can prove there is a performance bottleneck.

However, there is an OfType<T>() LINQ operator that you might find useful. Your first example, rewritten:

return _entries
    .Where(x => x.Context.Equals(context))
    .OfType<T>()
    .Select(x => (T)x.Value)
    .Single();

edited Mar 04 '13 at 09:53

answered Mar 01 '13 at 17:04

Daniel A.A. Pelsmaeker

47,471
20
111
157

To stay true to the example you would need a second `.Cast()` after `.Select(x => x.Value)` or to retain the original `.Select(x => (T)x.Value)`. Although I suspect it doesn't really matter because it is a made up example. – Alex Mar 01 '13 at 17:18
@AlexG No you wouldn't need that, as `OfType` returns an enumerable with all elements of type `T`. No need to cast any more. – Daniel A.A. Pelsmaeker Mar 01 '13 at 17:22
I encourage you to actually try the code and see if more enumerator objects are produced. You might be surprised. – Eric Lippert Mar 01 '13 at 17:47
@Virtlink In the original snippet, `x.Value` is cast to type `T`. In your example it is not. – Alex Mar 04 '13 at 09:02
@AlexG - Ah you're right. It is uncommon to have an object of type `T` with a property/field `Value` of type `T`, so I missed that. – Daniel A.A. Pelsmaeker Mar 04 '13 at 09:53

score 4 · Answer 3 · answered Mar 01 '13 at 17:14

4

In the long run the better coding practice is the one that is more readable. If you are concerned about performance, then test both approaches. If it's not worth testing then it's not worth optimizing.

answered Mar 01 '13 at 17:14

Dzienny

3,295
1
20
30

score 1 · Answer 4 · answered Mar 02 '13 at 07:13

I love Linq expression, as it gives a way to express the intent of your code declarativly. It focuses on what you want to achieve rather how to achieve it. So emphasis on the readability of your code.

But replacing some for/foreach blocks by Linq operators may also make the code cumbersome to realize. So I suggest to write your expression as verbose such that you are describing the logic verbally to your co-programmer, e.g. the below expression can be describe as "Return a Single value As of type T Where the entries context is equal to context and type is equal to T". If needed write custom extension methods, like the As<T>() that cast the value to type T.

return _entries
    .Where(x => x.Context.Equals(context) && x.Type == typeof (T))
    .Single (x=> x.Value)
    .As<T>();

Favour Verbose Linq expressions?

4 Answers4

Linked