What do you think of this improvement of Linq's GroupBy method?

Question

Linq already has a set of GroupBy methods for enumerable, which returns IEnumerable<IGrouping<TKey, TSource>>. I dislike this return type because it means I have to do a where query everytime I want to find a group. I try to make something that returns a dictionary instead, as below:

 public static class GroupByExtension
    {
        public static Dictionary<TKey, List<TSource>> GroupToDictionary<TSource, TKey>(
            this IEnumerable<TSource> source,
            Func<TSource, TKey> keySelector)
        {
            return source.GroupBy(keySelector).ToDictionary
                (grouping => grouping.Key, grouping => grouping.ToList());
        }
    }

I can test it like this

List<int> list = new List<int>() { 1, 2, 33, 4, 20, 43, 21, 93, 26, 31, 113 };
Dictionary<int, List<int>> numbersGroup = list.GroupToDictionary(i => i / 10); 
//group number by result of division by ten.

So, I can call numbersGroup[1], numbersGroup[3], etc. as opposed to the more clumsy and less efficient Where's clauses if I only have IEnumerable<IGrouping<TKey, TSource>>

IEnumerable<IGrouping<int, int>> numbersGroup2 = list.GroupBy(i => i / 10);
IGrouping<int, int> group =
            numbersGroup2.Where(grouping => grouping.Key == 3).First(); 
//and this only returns an IGrouping, not a proper list, 
//and where can be expensive for large list because it uses O(n) IIRC
//the first() could have also been avoided because we know that the keys are unique

What do you think of this code? What drawback do you see? And what is the reason Microsoft use IEnumerable<IGrouping<TKey, TSource>>

Answer 1

I think the reason it's done this way is to make GroupBy() consistent with the rest of LINQ: everything is as lazy as possible, so that following queries can be efficient.

And there already is a method that does almost the same thing as your GroupToDictionary(), it's called ToLookup().

Answer 2

Returning an interface type makes the software very future proof.

Even now, LINQ is works with any number of different providers, and each can implement the grouping in different ways, as long as the interface requirements are met. One example is that a LINQ provider can implement a lazy loaded grouping, which is not how the dictionary works.

Another option would be to create a class that implements IGrouping and gives you access through the this[] accessor.

Anyway, your solution is also quite good in my opinion. Just change the function return type to IDictionary instead of Dictionary, so you can change the implementation in the future.

Answer 3

The main reason for using IEnumerable<IGrouping<TKey, TValue>> would be defered loading/calculation, meaning that:

list.GroupBy(i => i / 10)

Would be almost instant, as no calculation and grouping are performed.
In your GroupToDictionary method the groupings are calculated when the method is called. So basically you are waiting in another place in the code.

The difference between your GroupToDictionary and the IEnumerable is that when a where is performed on the IEnumerable the calculation is performed when iterating over the value, and this is done each time the value is used as the result aren't cached. A common way to get around this is to either do ToDictionary, ToList, ToArray or any other "conversion" which enumerates a saves the result. This also gives you the performance you expect.

So basically I don't think there is anything wrong with you code, it the example provided you don't need defered loading, and that is one of the cases where I would do the same.