Reputation: 5119
Is it bad practice to declare LINQ directly in a foreach loop declaration? In terms of performance or subtle behavioral differences.
For example:
foreach (string name in persons.Select(x => x.name))
{
//Do something with name
}
Upvotes: 6
Views: 5942
Reputation: 10800
Bad practice? Not at all.
Performance could be impacted though, depending on the situation. For example, this:
persons.Select(x => x.name).Select(x => x.name + " more");
Could perform better than this:
foreach(string name in persons.Select(x => x.name))
{
someList.Add(name + " more");
}
...if you're using something like Entity Framework where the name + " more"
would happen on the database side vs local memory in the first example.
The best practice in my opinion is to go with whatever is most readable, then if you experience performance issues you can profile/tweak. Sometimes its clearer to use LINQ to build the IEnumerable, but the foreach to do some trickier stuff. If the LINQ part gets too long though, I would go with:
var somethings = something.Where(x => x == 1).Where(...).GroupBy(...etc);
foreach(var something in somethings)
{
// Do something
}
Upvotes: 3
Reputation: 149050
Nope. Nothing wrong with that. As long as the Linq expression is short and easily readable, I'd say that's the most important thing. Your example is a really good example of when such queries should be use that way.
If it get's much longer than that or if you use query syntax, however, I recommend splitting it into two lines like this:
var names = persons.Select(x => x.name).Blah().Blah().Blah();
foreach (string name in names)
{
//Do something with name
}
or
var names =
from x in persons
select x.name;
foreach (string name in names)
{
//Do something with name
}
Upvotes: 5