dotnet / corefxlab

This repo is for experimentation and exploring new ideas that may or may not make it into the main corefx repo.
MIT License
1.46k stars 345 forks source link

Add constructor for MultiDictionary that takes IEnumerable of KeyValuePair #2977

Closed adenner closed 3 years ago

adenner commented 4 years ago

This fixes the missing foreach in #2976 for sugguestion #2975

This would allow users to add an IEnumerable that is unsorted without presorting it.

There would be a performance hit, but would make for simpler code on the part of the user.

danmoseley commented 4 years ago

It's a good idea, the funny thing is that this type already went through API review years ago, so arguably changes should be as well if we plan to put it in the core libraries

https://devblogs.microsoft.com/dotnet/multidictionary-becomes-multivaluedictionary/

@terrajobst can you confirm this surface area (current area below) was approved? I am not sure where to go to check.

    public partial class MultiValueDictionary<TKey, TValue> : IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>>, IReadOnlyCollection<KeyValuePair<TKey, IReadOnlyCollection<TValue>>>, IReadOnlyDictionary<TKey, IReadOnlyCollection<TValue>>, IEnumerable
    {
        public MultiValueDictionary() { }
        public MultiValueDictionary(IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> enumerable) { }
        public MultiValueDictionary(IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> enumerable, IEqualityComparer<TKey> comparer) { }
        public MultiValueDictionary(IEqualityComparer<TKey> comparer) { }
        public MultiValueDictionary(int capacity) { }
        public MultiValueDictionary(int capacity, IEqualityComparer<TKey> comparer) { }
        public int Count { get { throw null; } }
        public IReadOnlyCollection<TValue> this[TKey key] { get { throw null; } }
        public IEnumerable<TKey> Keys { get { throw null; } }
        public IEnumerable<IReadOnlyCollection<TValue>> Values { get { throw null; } }
        public void Add(TKey key, TValue value) { }
        public void AddRange(TKey key, IEnumerable<TValue> values) { }
        public void Clear() { }
        public bool Contains(TKey key, TValue value) { throw null; }
        public bool ContainsKey(TKey key) { throw null; }
        public bool ContainsValue(TValue value) { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>() where TValueCollection : ICollection<TValue>, new() { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> enumerable) where TValueCollection : ICollection<TValue>, new() { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> enumerable, IEqualityComparer<TKey> comparer) where TValueCollection : ICollection<TValue>, new() { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> enumerable, IEqualityComparer<TKey> comparer, System.Func<TValueCollection> collectionFactory) where TValueCollection : ICollection<TValue> { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(IEnumerable<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> enumerable, System.Func<TValueCollection> collectionFactory) where TValueCollection : ICollection<TValue> { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(IEqualityComparer<TKey> comparer) where TValueCollection : ICollection<TValue>, new() { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(IEqualityComparer<TKey> comparer, System.Func<TValueCollection> collectionFactory) where TValueCollection : ICollection<TValue> { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(System.Func<TValueCollection> collectionFactory) where TValueCollection : ICollection<TValue> { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(int capacity) where TValueCollection : ICollection<TValue>, new() { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(int capacity, IEqualityComparer<TKey> comparer) where TValueCollection : ICollection<TValue>, new() { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(int capacity, IEqualityComparer<TKey> comparer, System.Func<TValueCollection> collectionFactory) where TValueCollection : ICollection<TValue> { throw null; }
        public static MultiValueDictionary<TKey, TValue> Create<TValueCollection>(int capacity, System.Func<TValueCollection> collectionFactory) where TValueCollection : ICollection<TValue> { throw null; }
        public IEnumerator<KeyValuePair<TKey, IReadOnlyCollection<TValue>>> GetEnumerator() { throw null; }
        public bool Remove(TKey key) { throw null; }
        public bool Remove(TKey key, TValue value) { throw null; }
        IEnumerator IEnumerable.GetEnumerator() { throw null; }
        public bool TryGetValue(TKey key, out IReadOnlyCollection<TValue> value) { value = default(IReadOnlyCollection<TValue>); throw null; }
    }
danmoseley commented 4 years ago

cc @eiriktsarpalis

danmoseley commented 4 years ago

BTW, I have no opinion whether it should go in the core libraries. It seems we have not gotten requests for that.

[edited to remove mistake]

danmoseley commented 4 years ago

maybe @stephentoub recalls

stephentoub commented 4 years ago

Nope

danmoseley commented 4 years ago

Incidentally I don't follow this

This would allow users to add an IEnumerable that is unsorted without presorting it.

Sort what? Why wouldn't someone, without this API, simply write exactly the code you wrote in this PR? Instead of

            var mvd = new MultiValueDictionary();
            foreach (var item in enumerable)
                mvd.Add(item.Key,item.Value);

they can now write

            var mvd = new MultiValueDictionary(enumerable);

You just save a couple lines.

adenner commented 4 years ago

You just save a couple lines.

I made the suggestion to add it after I noticed that I was having to write this code frequently. While not world changing it seemed like a convenience factor.

I could see arguments for instead having either a ToMultiDictionary linq method or a factory that will generate a Multi Dictionary instead.

adenner commented 4 years ago

I also realize that my pr is somewhat the naive implementation and there is possibly room for performance improvements.

eiriktsarpalis commented 4 years ago

I also realize that my pr is somewhat the naive implementation and there is possibly room for performance improvements.

What improvements could be made in this case?

adenner commented 4 years ago

Without further testing for larger datasets, I am uncertain if presorting/grouping before insertion would be a more performant solution as it would help prevent dictionary and list resizing costs.

On Tue, Oct 13, 2020 at 5:08 AM Eirik Tsarpalis notifications@github.com wrote:

I also realize that my pr is somewhat the naive implementation and there is possibly room for performance improvements.

What improvements could be made in this case?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dotnet/corefxlab/pull/2977#issuecomment-707637279, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACFMHZHTXDKIVKON5MFABFDSKQRIHANCNFSM4SNNY5LQ .