shapper said:
Yes, I would just also like to understand why my approach is not
working ... Just that.
If you have not understood my reply in the thread for your previous
question, you should ask for clarification in that thread. Be sure to
be state specifically and clearly what it is about the suggestion I
provided that you do not understand.
Maybe I didn't understand your question.
A Region can have many Centers. But one center has only one region.
In the case I am testing I have:
- 40 regions. Only one has centers associated with it: RegionId = 32.
- Region with Id=32 has 2 centers associated with it.
So I ended up with two records containing that region. I want only
one.
Maybe I am missing something?
Yes. As I explained before, your use of SelectMany() is completely
inappropriate. It specifically flattens the several enumerations of
Center instances into a single enumeration of Center instances, which
you then project to an enumeration of an anonymous type that contains,
for every Center instance: the Id for the Center's Region; the Name for
the Center's Region; and a collection of every Center referenced by that
Region where the Center's Open property is true.
So your result from the SelectMany() method is an enumeration of an
anonymous type that effectively copies any given Region instance once
for every Center that Region owns. You then eliminate every element of
that enumeration where the Region's Center collection contains no open
Centers, but otherwise the remaining Region instances are duplicated for
each Center a given Region contains.
All that was bad enough in the example you gave in your previous
question, but now you are specifically asking for the results to be
grouped by the Region instance when your data ALREADY has grouped the
data by Region. It is incredibly wasteful for you to write code that
strips that grouping away from the data, only then to have to add it
back in.
[...]
It's not really clear from your question what you really want and/or
need to happen, but it seems to me that if you want to do any grouping,
you should apply that grouping last. Alternatively, you can use the
SelectMany() method to flatting the IGrouping.
Lost about SelectMany(). I am already using it ...
Just because you're using it, that doesn't mean you're using it
correctly. You can't just toss in method calls wherever you like. They
have to be the correct method call in the correct place for the result
you want.
If you had really needed the SelectMany() method, it would have belonged
at the end, to flatten the output before returning. But as it turns
out, that's not really what you want anyway, at least not according to
your latest post.
And applying the GroupBy at the end was exactly what I tried first:
public IQueryable<Models.Region> FindWithCenters() {
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 Models.Region {
Id = o.Id,
Name = o.Name
}).GroupBy(p => p.Id);
}
But I get the error:
Cannot implicitly convert type
'System.Linq.IQueryable<System.Linq.IGrouping<int,Models.Region>>' to
'System.Linq.IQueryable<Models.Region>'.
An explicit conversion exists (are you missing a cast?)
The error seems pretty clear to me. The GroupBy() method returns one
type, and you have declared your method to return a completely different
type. Your method's return statement MUST return a value having the
same type as that declared for the method's return value.
Frankly, until you state clearly exactly what output you are looking to
get, there's no way to suggest a specific answer. However, based on the
method prototype you show above, in which you are trying to return an
enumeration of Region instances, it seems to me that the method is as
simple as this:
public IEnumerable<Models.Region> FindWithCenters()
{
return _context.Regions
.Where(r => r.Centers.Any(c => c.Open))
.OrderBy(r => r.Name);
}
That's it. You wanted an enumeration of Region instances that have a
non-empty Centers collection, and that's what the above does (I'll leave
the question of IQueryable vs IEnumerable to you…the Queryable class
provides a means to convert if your data provider isn't already
supporting that type).
Note that this is practically the same solution I provided in your
previous question. The only difference is that since you have
specifically stated in this most recent question that you want as a
result an actual enumeration of Region instances, I have simply left out
the projection to the anonymous type that you seemed to have wanted earlier.
You can, of course, actually return an IEnumerable<IGrouping<Region,
Center>> from your method if you like. And the simplest way to go about
doing something like that would in fact be to use the GroupBy() method.
But given that every Region instance already has an exact way to map
directly from the Region instance to its Center instances, why bother
with that?
Now, I will point out: your own code examples do not actually filter the
Center instances themselves. They examine the Center instances to
determine what Region instances have Centers with the Open property set
to "true". But that's all. You simply wind up with a list of Region
instances and do nothing with the Center instances past that.
That's not necessarily a problem. After all, with a list of Region
instances in hand like that, you can easily filter each Centers
collection according to the Open property again, as needed. But, if you
really want a result that is _only_ the Center instances that are open,
grouped by Region, the GroupBy() method is in fact a nice way to do
that. It would look something like this:
public IEnumerable<IGrouping<Region, Center>> FindWithCenters()
{
return _context.Regions
.SelectMany(r => r.Centers)
.Where(c => c.Open)
.GroupBy(c => c.Region, new RegionComparer());
}
where:
class RegionComparer : IEqualityComparer<Region>
{
public bool Equals(Region r1, Region r2)
{
return r1.Id == r2.Id;
}
public int GetHashCode(Region r)
{
return r.Id.GetHashCode();
}
}
If Region implements IComparable or if you are guaranteed that for any
given region ID, you have exactly one Region object instance allocated,
then you don't need the IEqualityComparer:
public IEnumerable<IGrouping<Region, Center>> FindWithCenters()
{
return _context.Regions
.SelectMany(r => r.Centers)
.Where(c => c.Open)
.GroupBy(c => c.Region);
}
Pete
