Create a Lookup<,> from a IEnumerable<IGrouping<,>>

user_14160477 0
Viewed 604

I'm not sure this question fits in StackOverflow. If that's the case, please let me know.

I'm trying to create a Lookup<,> from an IEnumerable<IGrouping<,>>, just for the sake of it (this isn't an XY problem).
My understanding is that the only way to create a Lookup object is with the ToLookup method.
The best way I found to do this is to separate the groupings into key-value pairs with duplicate keys and then group it again into a Lookup using ToLookup:

groups // IEnumerable<IGrouping<TKey, TElement>>
    .SelectMany(group => group.Select(item => new KeyValuePair<TKey, TElement>(group.Key, item)))
    .ToLookup(kvp => kvp.Key, kvp => kvp.Value)

I think this is very inefficient because it separates the groups and then 'reassembles' them, instead of taking advantage of the fact that they're already grouped.
Is there a better way to do this?

Possible use case:
Let's say we have a list of names. We want to group the names by their first letter, so far so good, but we only want to keep groups with more than two names, and we want the result to be a Lookup<,> so we'll have access to its useful indexer.
The first part can be done easily:

names.GroupBy(name => name[0]).Where(group => group.Count() > 2)

But then we will need to convert the IEnumerable<IGrouping<char, string>> to a Lookup<char, string>.

What reasons there are for not having a constructor equivalent to Dictionary<TKey, TValue>(IEnumerable<KeyValuePair<TKey, TValue>>)?

linq c#
4 Answers

In addition to the possible reasons that could explain why such functionality is not available that Marc pointed out, I just wanted to add that the indexer is also available in Dictionary, so you could create a IDictionary<char, IEnumerable<string>> and then keep in mind that you will get an Exception if you use the indexer with a key that's not in the dictionary (which is an important difference with the indexer in the ILookup... in addition to the Lookup being immutable in contrast to the dictionary).

So you could do something like this:

using System;
using System.Linq;
using System.Collections.Generic;

                    
public class Program
{
    public static void Main()
    {
        var names = new List<string>();
        
        names.Add("Agustin");   
        names.Add("Alejandro"); 
        names.Add("Diego"); 
        names.Add("Damian");
        names.Add("Dario");
        
        IDictionary<char, IEnumerable<string>> fakeLookup = names.GroupBy(name => name[0])
            .Where(group => group.Count() > 2)
            .ToDictionary(group => group.Key, group => group.AsEnumerable());
        
        foreach(var name in fakeLookup ['D'])
        {
            Console.WriteLine(name);
        }

        var namesStartingWithA = lookup['A']; // This will throw a KeyNotFoundException

    }
}
user_106488650

"What reasons there are for not having a constructor equivalent to..." - because every feature needs to be:

  1. thought of
  2. considered
  3. designed
  4. implemented
  5. tested
  6. documented
  7. supported

and either a) it didn't get to #1, or b) it was thought of, but got thrown out or deferred somewhere between #2 and #7, because either c) it was actively thought to be a bad idea, or d) it was a good-enough idea, but when compared to the sea of good ideas, it didn't meet the necessary threshold of benefit vs effort to get given the time to do it.

Marc_Gravell_233540

It is unclear to me why the Lookup<TKey, TValue> class is publicly exposed. This class has no public constructors, and it seems that there is no public API that returns this concrete type. The ToLookup LINQ operator returns an interface (ILookup<TKey, TValue>) instead of this type.

If you want to convert efficiently an IEnumerable<IGrouping<TKey, TValue>> to an ILookup<TKey, TValue>, without reconstructing the groupings from scratch, there seems to be no other option than writing a custom implementation of this interface. The implementation does not need to be public, and it's quite straightforward:

private class LookupOfGroupings<TKey, TValue> : ILookup<TKey, TValue>
{
    private readonly Dictionary<TKey, IGrouping<TKey, TValue>> _dictionary;

    public LookupOfGroupings(IEnumerable<IGrouping<TKey, TValue>> source) =>
        _dictionary = source.ToDictionary(g => g.Key);

    public int Count => _dictionary.Count;

    public IEnumerable<TValue> this[TKey key]
        => _dictionary.TryGetValue(key, out var g) ? g : Enumerable.Empty<TValue>();

    public bool Contains(TKey key) => _dictionary.ContainsKey(key);

    public IEnumerator<IGrouping<TKey, TValue>> GetEnumerator()
        => _dictionary.Values.GetEnumerator();

    IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();
}

The behavior of the indexer is the same with the behavior of the native implementation. In case of a non-existent key, it returns an empty sequence.

And here is the custom ToLookup operator that performs the conversion:

public static ILookup<TKey, TValue> ToLookup<TKey, TValue>(
    this IEnumerable<IGrouping<TKey, TValue>> source)
        => new LookupOfGroupings<TKey, TValue>(source);

Usage example:

ILookup<char, string> lookup = names
    .GroupBy(name => name[0])
    .Where(group => group.Count() > 2)
    .ToLookup();
user_111785490

The best way I found to do this is to separate the groupings into key-value pairs with duplicate keys and then group it again

If efficiency is the concern, I'm not really sure why you didn't just go straight to a lookup:

var look = names.ToLookup(n=> n[0], n => n);

Then you can ignore the entries that have less than 3 as you use the lookup.. If you'll be performing it a lot, make a method, local function or class to encapsulate the logic. You also mentioned about memory, but it's somewhat moot unless you get rid of names and just retain the lookup - the lookup doesn't contain a clone of all the names/you're not burning a huge amount extra by indexing these items you won't end up using. If you're after a truly efficient (for speed and memory) solution, don't use LiNQ

user_14106640
Related