Implement IReadOnlyDictionary

[Evengard]

With the help of Explicit Interface Implementation Should resolve #6

[SoftCircuits]

@Evengard One line of code in your request is this.

public IList<TKey> Keys => Items.Select(i => i.Key).ToList();

You like this because you say returning an IList has more reliable ordering over IEnumerable<T>. However, all this property does is take an IEnumerable<T> and converted to a list. If this property returned an IEnumerable<T>, the caller could simply convert it to a list and it would be the exact same thing.

I appreciate all of your input, but I've already made a change to support IReadOnlyDictionary<TKey, TValue> that involves changing the Keys and Values to return IEnumerable<T>. I think this is what the Microsoft classes do and so it would be more consistent. I've held off publishing my change because I'm willing to look at an example where this could cause issues. I can understand if you're done discussing it, but I at least wanted to give you a chance to show me such an example before I go ahead and publish my changes.

[Evengard]

This is mostly about contracts. Sure, iterating over a List won't change the order, but the contract itself (if we don't look at the implementation internals) doesn't guarantee that any ordering is preserved. We know that we're returning an IEnumerable which is based on a list, but for anyone not knowing theese internals this offers no such guarantees.

In my case I am using it with BouncyCastle for constructing certificates, where ordering is pretty important, and the contract of theese methods is explicitely requiring an IList, as an interface which guarantees ordering.

I am a strong believer that the contract of an object or method should make it clear it's intentions. Initially, when I started using the library, I was actually unsure if the Keys and Values are returned ordered as well, because nothing in the contract seemed to indicate it. Hence, the reason why I opened the issue and did the PR.

[SoftCircuits]

@Evengard One concern I have is that if all you're going to do is iterate over the list, then creating a List<T> requires heap allocations that can be expensive and must also be cleaned up by the garbage collector. It can be a performance hit that I'm still struggling to justify.

I don't really know about code contracts, although I read they are no longer supported in .NET Core. Do you know of any other way to specify that an IEnumerable<T> is returning items in order?

[Evengard]

Code contracts is more like a pattern than a language feature per se.

The memory allocation is actually a good thing here, as it provides a copy of the data here.

Imagine a case:

var dictionary = new OrderedDictionary<int, string>();

dictionary.Add(1, "first");

var values = dictionary.Values;

dictionary.Add(2, "second");

foreach(var value in values)
{
Console.WriteLine(value);
}

Here, without the said reallocation of the list if you just export the .Select() IEnumerable, you'll get in the console both "first" and "second"! That is because the enumeration - aka the .Select() application - happens only during the foreach (aka .Select() works lazily). I'm pretty sure people will be quite surprised by this behaviour. The list allocation guarantees that we get the Keys and Values valid at the time of the said Keys and Values querying.

[Evengard]

This actually gets worse. As if we continue after the said chunk of code with this:

dictionary.Add(3, "third");

foreach(var value in values)
{
Console.WriteLine(value);
}

We will get:

// First foreach
first
second
// Second foreach
first
second
third

Aka the IEnumerable is reiterated from scratch at each and every foreach from the beginning

[SoftCircuits]

@Evengard Well, that's certainly a valid consideration. Looking at Dictionary<TKey, TValue>, which was written before generics, these properties return KeyCollection and ValueCollection, which is basically ICollection<T>.

The additional overhead still bothers me but perhaps that's enough to do some variation on this.

Anyway, I appreciate you taking the time to discuss it.

[Evengard]

To be honest, I'd probably use an ImmutableList or even an ImmutableArray here, but I guess it would break the netstandard 2.0 compatibility...