Can I simplify this statement with a lambda expression?
var project = from a in accounts
from ap in a.AccountProjects
where ap.AccountProjectID == accountProjectId
select ap;
Add a comment
|
4 Answers
var project = accounts.SelectMany(a => a.AccountProjects)
.Where(x => x.AccountProjectID == accountProjectId);
Whether this is actually simpler is a matter of taste.
2 Comments
Joren
I think multiple from clauses are conceptually simpler than the implicitly-flattening SelectMany, but regardless of that, isn't the whole point of the LINQ syntax to give a cleaner syntax than these chained methods?
Mark
The two approaches are conceptually equivalent. They do the exact same thing. They are two different ways to express the exact same sets of operations. As I noted in my answer, which you find "cleaner" or "simpler" is a matter of taste.
Honestly, it looks pretty clear to me. I think that a lambda in this case may be less readable, i.e., something like Brandon posted below.
(Stolen from Brandon's post)
var project = accounts.Select(a => a.AccountProjects)
.Where(x => x.AccountProjectID == accountProjectId);
As far as readability is concerned, I think that a couple of loops is preferable to the lambda solution, and I think that your solution is preferable to the loops.
9 Comments
Joel Coehoorn
It depends on how you write it- add a line break before ".Where" and suddenly the lambda is actually pretty readable.
Brandon
I sort of agree with Ed. What you have now is perfectly readable, a lambda doesn't really simplify anything. This would probably be a matter of preference if anything.
Ed Swangren
Even with the line break, to me it is still less readable, but I guess that is a subjective thing. I guess the point is; it is perfectly clear, move on to a real problem! :)
Mark
Also, the lambda in this answer isn't correct, for the same reason that Brandon's isn't correct (you actually admitted you "stole" it from him).
Brandon
@Mark, I think you're partly missing the point of his post. He's not saying use a lambda and giving my (incorrect) answer as a solution, hes suggesting that switching from what he has now to a lambda would not give any readability benefit. (Well, depending on your preference)
|
I agree with Ed Swangren. This looks concise and readable enough.
Actually the answer to your question depends on 3 things:
- What you want to achieve - better readability? better performance? etc.
- The type of 'accounts'
- How the resulting collection is going to be used.
If you want better performance, and in case 'accounts' is a List, and the resulting collection will be iterated or passed to another method for iterating soon enough after these lines of code, I would do something like that:
List<Account> filteredAccounts = new List<Account>();
accounts.ForEach(a => { if (a.AccountProjectID == accountProjectId) filteredAccounts.Add(a); });
Surely it's less readable then your LINQ statement, but I would use these 2 lines rather than accounts.Select.......
And surely it's much better optimized for performance, which is always important I believe.
4 Comments
Jon Skeet
What's the benefit of that over calling
ToList() on the LINQ query?Mark
The question didn't ask you to optimize it, it asked you to simplify it. You're also making a pretty big assumption when you say it's "much better optimized for performance". Have you actually tested it? Do you have data to back up your assumption?
Alexander
1. What you call simplification - is a matter of taste. I think I answered this question in my previous post. 2. Of course I tested it. Othervise I wouldn't post it. The problem is not in calling ToList in the query. In average LINQ to objects is more than twice slowly than traditioal techniques (e.g. iterating using foreach cycle). Don't believe me? You can easily micro-benchmark it of just google it out. And ForEach extension method used here together with lambda expression gives you even better performance than foreach cycle. I put quite an effort into studying what things cost, believe me
Mark
You also made the same mistake as the other answers, by the way. Your code isn't at all equivalent to the OP's code. Account doesn't have an AccountProjectID property, it has a property called AccountProject, which is an IEnumerable<something> where something is a type that has an AccountProjectID property. I do agree with you that the LINQ stuff is slower in general, however.
accounts
.SelectMany (
a => AccountProjects,
(a, ct) =>
new
{
a = a,
ap = ap
}
)
.Where (t => (t.ap.AccountProjectID == t.a.accountProjectId))
.Select (t => t.ap)
