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

Add parameterless .ToDictionary() and .ToLookup() overloads to System.Linq.Enumerable #45679

Closed
andi0b opened this issue Dec 7, 2020 · 10 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq

Comments

@andi0b
Copy link

andi0b commented Dec 7, 2020

Background and Motivation

.ToDictionary() and .ToLookup are around for a long time and still very useful, sometimes it would be nicer if they could just be called without any parameters, if the usage implies what kind of dictionary/lookup is meant.

See also #24900, which proposes a subset of this suggestion (the KeyValuePair part)

// instead of writing
valueTupleEnumerable.ToDictionary(x=>x.key, x=>x.value)

// write
valueTupleEnumerable.ToDictionary();

Proposed API

Add additional extension methods to System.Linq.Enumerable

namespace System.Linq
{
    public static partial class Enumerable
    {
+        public static ILookup<TKey, TValue> ToLookup<TKey, TValue>(this IEnumerable<(TKey, TValue)> enumerable)
+            => enumerable.ToLookup(x => x.Item1, x => x.Item2);
+
+        public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<(TKey, TValue)> enumerable) where TKey : notnull
+            => enumerable.ToDictionary(x => x.Item1, x => x.Item2);
+
+        public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> enumerable) where TKey : notnull
+            => enumerable.ToDictionary(x => x.Key, x => x.Value);
    }
}

Usage Examples

record Person(int Id, string Name, int Age);

var persons = new List<Person> {
	new(1, "Person 1", 22),
	new(2, "Person 2", 22),
	new(3, "Person 3", 30),
	new(4, "Person 4", 40),
};

var personsAgeLookup = (
	from person in persons
	select (key: person.Age, value: person.Name)
).ToLookup();

var sameAgedPersonLookup = (
	from person in persons
	from sameAgedPerson in personsAgeLookup[person.Age]
	where sameAgedPerson != person.Name
	select (person, sameAgedPerson)
).ToLookup();

var dict = new Dictionary<int,Person>();
var newDict = dict.Where(i=>i.Key*10 == i.Value.Age).ToDictionary();

instead of

var personsAgeLookup = (
	from person in persons
	select (key: person.Age, value: person.Name)
).ToLookup(x=>x.key, x=>x.value);

var sameAgedPersonDictionary = (
	from person in persons
	from sameAgedPerson in personsAgeLookup[person.Age]
	where sameAgedPerson != person.Name
	select (person, sameAgedPerson)
).ToDictionary(x=>x.person, x=>x.sameAgedPerson);

var dict = new Dictionary<int, Person>();
var newDict = new Dictionary<int,Person>(dict.Where(i => i.Key * 10 == i.Value.Age));

Alternative Designs

  • Don't know...
  • Maybe find other functions with similar design issue.
  • Also support Tuple<T1,T2>?
  • Instead of creating new overloads, maybe call them AsDictionary and AsLookup?

Risks

none?

@andi0b andi0b added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Linq untriaged New issue has not been triaged by the area owner labels Dec 7, 2020
@ghost
Copy link

ghost commented Dec 7, 2020

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

.ToDictionary() and .ToLookup are around for a long time and still very useful, sometimes it would be nicer if they could just be called without any parameters, if the usage implies what kind of dictionary/lookup is meant.

See also #24900, which proposes a subset of this suggestion (the KeyValuePair part)

// instead of writing
valueTupleEnumerable.ToDictionary(x=>x.key, x=>x.value)

// write
valueTupleEnumerable.ToDictionary();

Proposed API

Add additional extension methods to System.Linq.Enumerable

namespace System.Linq
{
    public static partial class Enumerable
    {
+        public static ILookup<TKey, TValue> ToLookup<TKey, TValue>(this IEnumerable<(TKey, TValue)> enumerable)
+            => enumerable.ToLookup(x => x.Item1, x => x.Item2);
+
+        public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<(TKey, TValue)> enumerable) where TKey : notnull
+            => enumerable.ToDictionary(x => x.Item1, x => x.Item2);
+
+        public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> enumerable) where TKey : notnull
+            => enumerable.ToDictionary(x => x.Key, x => x.Value);
    }
}

Usage Examples

record Person(int Id, string Name, int Age);

var persons = new List<Person> {
	new(1, "Person 1", 22),
	new(2, "Person 2", 22),
	new(3, "Person 3", 30),
	new(4, "Person 4", 40),
};

var personsAgeLookup = (
	from person in persons
	select (key: person.Age, value: person.Name)
).ToLookup();

var sameAgedPersonLookup = (
	from person in persons
	from sameAgedPerson in personsAgeLookup[person.Age]
	where sameAgedPerson != person.Name
	select (person, sameAgedPerson)
).ToLookup();

var dict = new Dictionary<int,Person>();
var newDict = dict.Where(i=>i.Key*10 == i.Value.Age).ToDictionary();

instead of

var personsAgeLookup = (
	from person in persons
	select (key: person.Age, value: person.Name)
).ToLookup(x=>x.key, x=>x.value);

var sameAgedPersonDictionary = (
	from person in persons
	from sameAgedPerson in personsAgeLookup[person.Age]
	where sameAgedPerson != person.Name
	select (person, sameAgedPerson)
).ToDictionary(x=>x.person, x=>x.sameAgedPerson);

var dict = new Dictionary<int, Person>();
var newDict = new Dictionary<int,Person>(dict.Where(i => i.Key * 10 == i.Value.Age));

Alternative Designs

  • Don't know...
  • Maybe find other functions with similar design issue.
  • Also support Tuple<T1,T2>?
  • Instead of creating new overloads, maybe call them AsDictionary and AsLookup?

Risks

none?

Author: andi0b
Assignees: -
Labels:

api-suggestion, area-System.Linq, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

Hi @andi0b and thank you for your proposal.

How common is it that the source enumerable is of type KeyValuePair or ValueTuple? In both examples that you gave you are inserting an additional select statement to map the source type into one that works with the proposed overloads, so it's not clear to me how this would simplify existing workflows.

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Dec 7, 2020
@canton7
Copy link

canton7 commented Dec 7, 2020

I do this a fair amount (the use-case is normally constructing a KVP somewhere earlier in the linq query, then perhaps filtering it), but then pass the resulting IEnumerable<KVP> to the Dictionary ctor to avoid the extra pointless delegate calls from .ToDictionary(x => x.Key, x => x.Value).

Adding this would save me a few keystrokes, but there's a perfectly serviceable way of writing this today, so it's not a big deal.

@BhaaLseN
Copy link

BhaaLseN commented Dec 7, 2020

I also see .ToDictionary(x => x.Key, x => x.Value) pretty frequently, which is often (historically) KeyValuePair<TKey, TValue>, (later) anonymous types with two members and (recently) ValueTuple<T1, T2>.
No big deal, but if those can be determined automatically I wouldn't mind saving a few keystrokes here and there.

@scalablecory
Copy link
Contributor

This may be a better fit for System.Interactive.

@andi0b
Copy link
Author

andi0b commented Dec 7, 2020

@eiriktsarpalis: I totally agree that this API change is more a nice to have, than a must. But it can save some typing. It can simplify many generation of a Dictionary or Lookup from LINQ queries.

I know there is also a constructor Dictionary<TKey,TValue>(IEnumerable<KeyValuePair<TKey,TValue>>) but using it, it breaks with method chaining. A constructor like that does not exist for ILookup though.

Like @BhaaLseN already said, I also see .ToDictionary(x => x.Key, x => x.Value) quite often, and often it is also a anonymous class. Sadly for anonymous classes I see no possibility to implement a parameterless ToDictionary().

@eiriktsarpalis: You're right, that my first example (personsAgeLookup) is too simple to get any benefit, consider this example instead:

// query syntax
var personsAgeLookup = (
	from person in persons
	let key = GetKey(person)
	where key % 2 == 0
	select (key, value: person.Name)
).ToLookup(); // instead of .ToLookup(x=>x.key, x=>x.person)

// or method syntax
var personsAgeLookup2 = persons.Select(person=>(key: GetKey(person), person))
         .Where(x=>x.key % 2 == 0)
         .ToLookup(); // instead of .ToLookup(x=>x.key, x=>x.person)

@andi0b
Copy link
Author

andi0b commented Dec 7, 2020

@scalablecory: I see no reason why this feature should go into System.Interactive. If it is accepted it should be available for LINQ everywhere.

@eiriktsarpalis
Copy link
Member

If it is accepted it should be available for LINQ everywhere.

Adding new methods to LINQ comes at a cost: shared framework size. We generally don't do it unless we are able to gather substantial evidence that it provides real value to a big chunk of our users. The nice thing about LINQ is that it is extensible: just write an extension method of your own and you should be set. Even better, author a LINQ extensions nuget package that other users can enjoy. From a .NET team perspective we fully encourage community members building successful NuGet packages for the ecosystem.

@andi0b
Copy link
Author

andi0b commented Dec 8, 2020

@eiriktsarpalis: totally understandable if this is declined because it will just bloat the framework unnecessarily.

I just always get annoyed by this particular missing functionality. And I try not to use non-standard Linq methods/extensions, because code is less readable then. The reader may not know the particular library I used and has to look it up.

@eiriktsarpalis
Copy link
Member

Thank you for understanding. I'm going to close this issue but as always feel free to reopen if you feel there is more to be added to this conversation.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq
Projects
None yet
Development

No branches or pull requests

6 participants