dotnet icon indicating copy to clipboard operation
dotnet copied to clipboard

Add backwards compatibility for ObservableGroupedCollection extensions.

Open tinodo opened this issue 7 months ago • 1 comments

Overview

Whilst upgrading from CommunityToolkit.Common.Collections to CommunityToolkit.Mvvm, I noticed that a lot of (linq e.a.) extensions on ObservableGroupedCollection and derived classes I am using in my code are no longer available.

For example;

collection.OrderBy collection.Sum collection.FirstOrDefault collection.Select collection.SelectMany

It appears as if a previously implemented interface (IEnumerable?) is no longer implemented.

Can this be added 'back' please? I can't find any other viable workarounds.

API breakdown

I'm not able to provide an API breakdown.

Usage example

var all = this._groupedRegistrations.SelectMany(x => x).ToList();
var location = this._groupedRegistrations.Select(x => x.Key).Append(groupCharacter).OrderBy(x => x).TakeWhile(x => x != groupCharacter).Count();
var cnt = this.GroupedRegistrations.Sum(g => g.Count);

Breaking change?

No

Alternatives

At this moment, I am using the old version of the CommunityToolkit as a workaround.

Help us help you

No, just wanted to propose this

tinodo avatar Nov 20 '23 12:11 tinodo

The methods are supposed to still be there, but it seems https://github.com/CommunityToolkit/dotnet/commit/224b3342e5baf006844ebee9304385d641565e4e broke the binding for LINQ extensions due to the second IEnumerable<T> implementation being present on those grouped collection types, which the compiler cannot resolve on its own. This is unfortunate. As a temporary workaround, you should be able to call all of those LINQ extensions either explicitly, or by casting to the enumerable type first. That is:

ObservableGroupedCollection<string, object> groupedRegistrations = new();

// Explicit extension invocation
var a = Enumerable.First<ObservableGroup<string, object>>(groupedRegistrations);

// Casting first
var b = ((IEnumerable<ObservableGroup<string, object>>)groupedRegistrations).First();

I think the right way to fix this is likely to remove that ILookup<TKey, TElement> implementation, and instead add an AsLookup method that returns a thin wrapper over the collection, implementing just ILookup<TKey, TElement> 🤔

Of course, that's a breaking change. We should consider it for 9.0.

Sergio0694 avatar Nov 20 '23 13:11 Sergio0694