13
\$\begingroup\$

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>>

\$\endgroup\$
18
\$\begingroup\$

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().

\$\endgroup\$
  • \$\begingroup\$ ah, never heard of that, is look up similar to IDictionary<TKey, List<TValue>>? \$\endgroup\$ – Louis Rhys Jul 4 '12 at 7:42
  • \$\begingroup\$ Yes, thanks to its indexer, it behaves close to Dictionary<TKey, IEnumerable<TValue>> (it doesn't actually implement that interface). \$\endgroup\$ – svick Jul 4 '12 at 8:20
  • 1
    \$\begingroup\$ nice.. does it have deferred/lazy loading? (just curious, not that it matters for me) \$\endgroup\$ – Louis Rhys Jul 4 '12 at 8:24
  • 4
    \$\begingroup\$ No, all of the ToXxx() methods don't use lazy loading or deferred execution. \$\endgroup\$ – svick Jul 4 '12 at 8:28
  • \$\begingroup\$ Also, GroupBy uses Lookup to get the groupings referencesource.microsoft.com/#System.Core/System/Linq/… \$\endgroup\$ – Slai Jan 11 '17 at 4:39
6
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ But that doesn't explain why doesn't GrouBy() return something like IDictionary<TKey, IList<TSource>>, that's an interface type and could work with any provider. \$\endgroup\$ – svick Jul 4 '12 at 7:36
  • \$\begingroup\$ Well, a LINQ provider could e.g. implement the grouping with lazy loading. That wouldn't work with a dictionary. IDictionary and IGrouping represent different things and should be represented differently \$\endgroup\$ – Schiavini Jul 4 '12 at 7:38
  • \$\begingroup\$ ah I see.. what do you mean by "providers"? Also do you think I should change List to IList too? \$\endgroup\$ – Louis Rhys Jul 4 '12 at 7:40
  • \$\begingroup\$ Provider I mean that the LINQ could be interfacing with XML, SQL, objects, or anything else. It's always a good idea to use the interfaces indeed, IList is a good example. \$\endgroup\$ – Schiavini Jul 4 '12 at 11:24
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ interesting.. But how could group by be yielded one by one? the grouper code will need to iterate one by one before returning the first value anyway right? \$\endgroup\$ – Louis Rhys Jul 4 '12 at 9:40
  • \$\begingroup\$ @LouisRhys That is correct, the groups are created before returning the values. The difference is when the groups are created. \$\endgroup\$ – Xharze Jul 4 '12 at 10:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.