shapper said:
[...]
And I came up with the following, which is working:
return _context.Regions.SelectMany(r => r.Centers, (r, l) => new
{ r.Id, r.Name, Centers = r.Centers.Where(c => c.Open == true) })
.Where(o => o.Centers.Count() > 0)
.OrderBy(o => o.Name)
.Select(o => new ModelRegion {
Id = o.Id,
Name = o.Name
});
What do you think? Maybe it could be improved ... The "l" is not doing
anything ...
Yes, I think the fact you're not using "l" does suggest an improper use
of the SelectMany() method.
Your final enumeration from that method
is a collection of every Region represented potentially multiple times,
once for each Center that refers to it. That doesn't seem very
efficient to me, especially since you don't do anything later to deal
with the duplicate Region instances.
(By the way, not relevant here because the rest of the query needs
fixing anyway, but the expression "o.Centers.Count() > 0" really should
be "o.Centers.Any()"…counting the entire enumeration is potentially much
more expensive than just seeing if it's non-empty).
Instead, if you're going to approach it from the direction of the Region
objects, what you really want are just Region instances that have any
Center related to it that is open. That would look more like this:
var result = _context.Regions
.Where(r => r.Centers.Any(c => c.Open))
.OrderBy(r => r.Name)
.Select(r => new ModelRegion
{
Id = r.Id,
Name = r.Name
});
Or using the LINQ syntax:
var result = from region in _context.Regions
where region.Centers.Any(c => c.Open)
orderby region.Name
select new ModelRegion { Id = region.Id, Name = region.Name };
I'm not completely sure I understand the data. I'm not a SQL expert, so
forgive me if I missed the point.
My assumption in the above is that any given Center has only one Region.
Thus, looking at each Region, you visit each Center at most once, and
may avoid visiting some Centers, if they are related to a Region that
has at least one other Center that's "open" and which comes before it in
the Region's collection of Center instances.
If that assumption isn't valid, then it may be more efficient to weed
out all the Center instances that aren't "open" first, and then figure
out the unique Region instances associated with those. That might look
something like this:
var result = from region in
(from center in _context.Centers
where center.Open
select new
{
Id = center.Region.Id,
Name = center.Region.Name
})
.Distinct()
orderby region.Name
select new ModelRegion { Id = region.Id, Name = region.Name };
Pete