Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please introduce a reusable implementation of ILookup #23296

Closed
weitzhandler opened this issue Aug 22, 2017 · 5 comments
Closed

Please introduce a reusable implementation of ILookup #23296

weitzhandler opened this issue Aug 22, 2017 · 5 comments
Milestone

Comments

@weitzhandler
Copy link
Contributor

Hi,

The Lookup class' constructor is private, and its Create methods are internal.
I'd like to request to make the Create methods in lookup public, and/or introduce a public/protected constructor to it, so we can benefit of having custom Lookup implementations.

Alternatively (preferable option), please provide a comprehensive solution to deal with mutable multi-value dictionaries (i.e. single key with multiple values).
The MultiValueDictionary in the corefxlab repo, implements IReadOnlyDictionary, which is immutable. I'd prefer a permanent editable multi-value dictionary.
Here's a shallow way of how I achieve this, but it'll only work if the keys are immutable (just like KeyedCollection itself will).

Gist

private abstract class Lookup<TKey, TElement> : KeyedCollection<TKey, ICollection<TElement>>
{
  protected override TKey GetKeyForItem(ICollection<TElement> item) =>
    item
    .Select(b => GetKeyForItem(b))
    .Distinct()
    .SingleOrDefault();

  protected abstract TKey GetKeyForItem(TElement item);

  public void Add(TElement item)
  {
    var key = GetKeyForItem(item);
    if (Dictionary != null && Dictionary.TryGetValue(key, out var collection))
      collection.Add(item);
    else
      Add(new List<TElement> { item });
  }

  public void Remove(TElement item)
  {
    var key = GetKeyForItem(item);
    if (Dictionary != null && Dictionary.TryGetValue(key, out var collection))
    {
      collection.Remove(item);
      if (collection.Count == 0)
        Remove(key);
    }
  }
}
@danmoseley
Copy link
Member

@weitzhandler hello, please format your proposal as per example linked here https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

@weitzhandler
Copy link
Contributor Author

@danmosemsft updated

@weitzhandler weitzhandler changed the title Allow public usage of Linq Lookups Please introduce a public reusable implementation of ILookup Aug 22, 2017
@weitzhandler weitzhandler changed the title Please introduce a public reusable implementation of ILookup Please introduce a reusable implementation of ILookup Aug 22, 2017
@danmoseley
Copy link
Member

danmoseley commented Aug 22, 2017

@weitzhandler please present the API in reference format similar to Proposed API in https://github.com/dotnet/corefx/issues/271. Also it's good to break out Motivation, and Potential Alternatives. In some cases Performance is also worth breaking out. You already have an Example of use which is good.
I know it's paperwork but it makes it much easier to formally review to have sections organized.

@danmoseley
Copy link
Member

You can copy/paste relevant part of https://github.com/dotnet/corefx/blob/master/src/System.Linq/ref/System.Linq.cs and add in your API.

@weitzhandler
Copy link
Contributor Author

weitzhandler commented Aug 22, 2017

I'm sorry but I currently don't have the time to put it all up like that.
My motive was indeed to simply allow reusing the Lookup code rather than having to copy paste it.
I'm gonna close this issue.

Thanks for your effort @danmosemsft.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants