Extension methods for both IDictionary and IReadOnlyDictionary

Question

My question is similar to the previous one, but the answer to that question is inapplicable to this one.

Well, I want to write an extension method for both IDictionary and IReadOnlyDictionary interfaces:

public static TValue? GetNullable<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary, TKey key)
    where TValue : struct
{
    return dictionary.ContainsKey(key)
        ? (TValue?)dictionary[key]
        : null;
}

public static TValue? GetNullable<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key)
    where TValue : struct
{
    return dictionary.ContainsKey(key)
        ? (TValue?)dictionary[key]
        : null;
}

But when I use it with classes implementing both interfaces (e.g. Dictionary<Tkey, TValue>), I get the ‘ambiguous invocation’. I don't want to type var value = myDic.GetNullable<IReadOnlyDictionary<MyKeyType, MyValueType>>(key), I want it to be just var value = myDic.GetNullable(key).

Is this possible?

Answer 1

You only need one extension method. Redefine it as follows:

public static TValue? GetNullableKey<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> dictionary, TKey, key) where TValue : struct
{
    // Your code here.
    // Per @nmclean: to reap performance benefits of dictionary lookup, try to cast
    //               dictionary to the expected dictionary type, e.g. IDictionary<K, V> or
    //               IReadOnlyDictionary<K, V>. Thanks for that nmclean!
}

Both IDictionary<TKey, TValue> and IReadOnlyDictionary<TKey, TValue> inherit from IEnumerable<KeyValuePair<TKey, TValue>>.

HTH.

UPDATE: To answer your comment question

If you don't need to call methods that only appear on one interface or the other (i.e. you're only calling methods that exist on IEnumerable<KeyValuePair<TKey, TValue>>), then no, you don't need that code.

If you do need to call methods on either the IDictionary<TKey, TValue> or IReadOnlyDictionary<TKey, TValue> interface that don't exist on the common base interface, then yes, you'll need to see if your object is one or the other to know which methods are valid to be called.

UPDATE: You probably (most likely) shouldn't use this solution

So, technically, this solution is correct in answering the OP's question, as stated. However, this is really not a good idea, as others have commented below, and as I have acknowledged as correct those comments.

For one, the whole point of a dictionary/map is O(1) (constant time) lookup. By using the solution above, you've now turned an O(1) operation into an O(n) (linear time) operation. If you have 1,000,000 items in your dictionary, it takes up to 1 million times longer to look up the key value (if you're really unlucky). That could be a significant performance impact.

What about a small dictionary? Well, then the question becomes, do you really need a dictionary? Would you be better off with a list? Or perhaps an even better question: where do you start to notice the performance implications of using an O(n) over O(1) lookup and how often do you expect to be performing such a lookup?

Lastly, the OP's situation with an object that implements both IReadOnlyDictionary<TKey, TValue> and IDictionary<TKey, TValue> is, well, odd, as the behavior of IDictionary<TKey, TValue> is a superset of the behavior of IReadOnlyDictionary<TKey, TValue>. And the standard Dictionary<TKey, TValue> class already implements both of these interfaces. Therefore, if the OP's (I'm assuming, specialized) type had inherited from Dictionary<TKey, TValue> instead, then when read-only functionality was required, the type could've simply been cast to an IReadOnlyDictionary<TKey, TValue>, possibly avoiding this whole issue altogether. Even if the issue was not unavoidable (e.g. somewhere, a cast is made to one of the interfaces), it would still be better to have implemented two extension methods, one for each of the interfaces.

I think one more point of order is required before I finish this topic. Casting a Dictionary<TKey, TValue> to IReadOnlyDictionary<TKey, TValue> only ensures that whatever receives the casted value will not itself be able to mutate the underlying collection. That does not mean, however, that other references to the dictionary instance from which the casted reference was created won't mutate the underlying collection. This is one of the gripes behind the IReadOnly* collection interfaces—the collections referenced by those interfaces may not be truly "read-only" (or, as often intimated, immutable) collections, they merely prevent mutation of the collection by a particular reference (notwithstanding an actual instance of a concrete ReadOnly* class). This is one reason (among others) that the System.Collections.Immutable namespace of collection classes were created. The collections in this namespace represent truly immutable collections. "Mutations" of these collections result in an all-new collection being returned consisting of the mutated state, leaving the original collection unaffected.

Answer 2

After some experimenting, I remembered/found that C# is willing to resolve ambiguities by preferring subclasses over base classes. Thus, to do this with type safety, you need 3 extension methods rather than one. And if you have any other classes which, like Dictionary<TKey, TValue>, implement both IDictionary<TKey, TValue> and IReadOnlyDictionary<TKey, TValue>, you need to write more and more extension methods…

The following makes the compiler perfectly happy. It does not require casting to use the passed dictionary as a dictionary. My strategy is to implement my actual code for the lowest common denominator, IReadOnlyDictionary<TKey, TValue>, use a façade to adapt IDictionary<TKey, TValue> to that (because you could be passed an object which doesn’t implement IReadOnlyDictionary<TKey, TValue>), and, to avoid the method overload resolution ambiguity error, explicitly extend Dictionary<TKey, TValue> directly.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;

// Proof that this makes the compiler happy.
class Program
{
    static void Main(string[] args)
    {
        // This is written to try to demonstrate an alternative,
        // type-clean way of handling http://stackoverflow.com/q/18641693/429091
        var x = "blah".ToDictionary(
            c => c,
            c => (int)c);
        // This would be where the ambiguity error would be thrown,
        // but since we explicitly extend Dictionary<TKey, TValue> dirctly,
        // we can write this with no issue!
        x.WriteTable(Console.Out, "a");
        IDictionary<char, int> y = x;
        y.WriteTable(Console.Out, "b");
        IReadOnlyDictionary<char, int> z = x;
        z.WriteTable(Console.Out, "lah");
    }
}

// But getting compile-time type safety requires so much code duplication!
static class DictionaryExtensions
{
    // Actual implementation against lowest common denominator
    public static void WriteTable<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dict, TextWriter writer, IEnumerable<TKey> keys)
    {
        writer.WriteLine("-");
        foreach (var key in keys)
            // Access values by key lookup to prove that we’re interested
            // in the concept of an actual dictionary/map/lookup rather
            // than being content with iterating over everything.
            writer.WriteLine($"{key}:{dict[key]}");
    }

    // Use façade/adapter if provided IDictionary<TKey, TValue>
    public static void WriteTable<TKey, TValue>(this IDictionary<TKey, TValue> dict, TextWriter writer, IEnumerable<TKey> keys)
        => new ReadOnlyDictionary<TKey, TValue>(dict).StaticCast<IReadOnlyDictionary<TKey, TValue>>().WriteTable(writer, keys);

    // Use an interface cast (a static known-safe cast).
    public static void WriteTable<TKey, TValue>(this Dictionary<TKey, TValue> dict, TextWriter writer, IEnumerable<TKey> keys)
        => dict.StaticCast<IReadOnlyDictionary<TKey, TValue>>().WriteTable(writer, keys);

    // Require known compiletime-enforced static cast http://stackoverflow.com/q/3894378/429091
    public static T StaticCast<T>(this T o) => o;
}

The most annoying thing with this approach is that you have to write so much boilerplate—two thin wrapper extension methods and one actual implementation. However, I think this can be justified because code using the extensions can be kept simple and clear. The ugliness is contained in the the extensions class. Also, because the extension implementation is contained in one method, you do not need to worry about updating the overloads when the IReadOnlyDictionary<TKey, TValue> variant is updated. The compiler will even remind you to update the overloads if you change the parameter types unless you’re adding new parameters with default values.

Answer 3

To get the desired behavior in your case, see fourpastmidnight's answer.

---

To directly answer your question though, no, it isn't possible to make the myDic.GetNullable syntax work when there are two extension methods. The error message you got was correct in saying it's ambiguous, because it has no way of knowing which one to call. If your IReadOnlyDictionary method happened to do something different, should it do that or not for Dictionary<TKey, TValue>?

It's for this reason that the AsEnumerable method exists. Extension methods are defined for both IEnumerable and IQueryable with the same names, so you sometimes need to cast objects that implement both to ensure that a specific extension method will be called.

Supposing your two methods did have different implementations, you could include similar methods like this:

public static IReadOnlyDictionary<TKey, TValue> AsReadOnly<TKey, TValue>(this IDictionary<TKey, TValue> dictionary) {
    return (IReadOnlyDictionary<TKey, TValue>)dictionary;
}

public static IDictionary<TKey, TValue> AsReadWrite<TKey, TValue>(this IReadOnlyDictionary<TKey, TValue> dictionary) {
    return (IDictionary<TKey, TValue>)dictionary;
}

Then call the methods like this:

dictionary.AsReadOnly().GetNullable(key);
dictionary.AsReadWrite().GetNullable(key);

(Technically, a method named AsReadOnly should return a real ReadOnlyDictionary, but this is just for demonstration)