7

Nostalgic for Collections.unmodifiableMap(), I've been implementing a read-only IDictionary wrapper based on this discussion, and my unit test quickly ran into a problem:

Assert.AreEqual (backingDictionary, readOnlyDictionary);

fails, even though the key-value pairs match. I played around a little more, and it looks like at least (thank Simonyi)

Assert.AreEquals (backingDictionary, new Dictionary<..> { /* same contents */ });

does pass.

I took a quick look through the Dictionary and IDictionary documentation, and to my surprise I couldn't find any equivalent of the Java Map contract that two Maps with equal entrySet()s must be equal. (The docs say that Dictionary -- not IDictionary -- overrides Equals(), but don't say what that override does.)

So it looks like key-value equality in C# is a property of the Dictionary concrete class, not of the IDictionary interface. Is this right? Is it generally true of the whole System.Collections framework?

If so, I'd be interested to read some discussion of why MS chose that approach -- and also of what the preferred way would be to check for equality of collection contents in C#.

And finally, I wouldn't mind a pointer to a well-tested ReadOnlyDictionary implementation. :)


ETA: To be clear, I'm not looking for suggestions on how to test my implementation -- that's relatively trivial. I'm looking for guidance on what contract those tests should enforce. And why.


ETA: Folks, I know IDictionary is an interface, and I know interfaces can't implement methods. It's the same in Java. Nevertheless, the Java Map interface documents an expectation of certain behavior from the equals() method. Surely there must be .NET interfaces that do things like this, even if the collection interfaces aren't among them.

Community
  • 1
  • 1
David Moles
  • 48,006
  • 27
  • 136
  • 235
  • There's a ReadOnlyDictionary in the MS.Internal.Utility namespace in the WindowsBase assembly. It doesn't override Equals. – dtb Oct 12 '10 at 22:03
  • 1
    I'm writing a Mono app for iOS, but that's an interesting data point. Are two `MS.Internal.Utility.ReadOnlyDictionaries` with the same contents equal to each other? – David Moles Oct 12 '10 at 22:06
  • 1
    Noted for later readers: (1) the `Algorithms` class in [PowerCollections](http://powercollections.codeplex.com/) provides `ReadOnly` methods for wrapping collections (including dictionaries) as read-only. (2) LINQ's [SequenceEqual()](http://msdn.microsoft.com/en-us/library/bb348567.aspx) works on ordered collections (including dictionaries). – David Moles Jun 23 '11 at 23:48

6 Answers6

3

Overriding equals is normally only done with classes which have a degree of value semantics (e.g. string). Reference equality is what people are more often concerned about with most reference types and a good default, especially in cases which can be less than clear (are two dictionaries with exactly the same key-value-pairs but different equality-comparers [and hence adding the same extra key-value-pair could make them now different] equal or not?) or where value-equality is not going to be frequently looked for.

After all, you are looking for a case where two different types are considered equal. An equality override would probably still fail you.

All the more so as you can always create your own equality comparer quickly enough:

public class SimpleDictEqualityComparer<TKey, TValue> : IEqualityComparer<IDictionary<TKey, TValue>>
{
    // We can do a better job if we use a more precise type than IDictionary and use
    // the comparer of the dictionary too.
    public bool Equals(IDictionary<TKey, TValue> x, IDictionary<TKey, TValue> y)
    {
        if(ReferenceEquals(x, y))
            return true;
        if(ReferenceEquals(x, null) || ReferenceEquals(y, null))
            return false;
        if(x.Count != y.Count)
            return false;
        TValue testVal = default(TValue);
        foreach(TKey key in x.Keys)
            if(!y.TryGetValue(key, out testVal) || !Equals(testVal, x[key]))
                return false;
        return true;
    }
    public int GetHashCode(IDictionary<TKey, TValue> dict)
    {
        unchecked
        {
            int hash = 0x15051505;
            foreach(TKey key in dict.Keys)
            {
                var value = dict[key];
                var valueHash = value == null ? 0 : value.GetHashCode();
                hash ^= ((key.GetHashCode() << 16 | key.GetHashCode() >> 16) ^ valueHash);
            }
            return hash;
        }
    }
}

That wouldn't serve all possible cases where one wants to compare dictionaries, but then, that was my point.

Filling up the BCL with "probably what they mean" equality methods would be a nuisance, not a help.

nawfal
  • 70,104
  • 56
  • 326
  • 368
Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
  • The equality-comparer adds an extra wrinkle that Java doesn't have, I admit. – David Moles Oct 12 '10 at 22:43
  • @David, Java has some way of defining an external definition of equivalence between objects though, surely? – Jon Hanna Oct 12 '10 at 22:46
  • Yes and no. There's `Comparator`, and if your implementation of `Comparator.compareTo(a, b)` returns 0, `a` and `b` are supposed to be equal. Usually nobody discovers this until they construct a `TreeSet` or `TreeMap` (the default sorted implementations) and members start falling out of it due to a bad `Comparator` implementation, though. There isn't really anything like `IEqualityComparer` in the standard library -- collections just use `equals()`. (Except, as noted, `TreeMap/TreeSet`, which take a shortcut.) – David Moles Oct 12 '10 at 22:55
  • 1
    @Jon: A small nitpick: Your `GetHashCode` method is dependent on the ordering of `dict.Keys`, and that ordering isn't guaranteed in any way. Two `IDictionary` instances containing the same set of key-value pairs could, in theory, produce different hashcodes even though they compare as equal. – LukeH Oct 12 '10 at 23:02
  • Ah, I take it then that Comparator doesn't support the sort of weak ordering where things could sort in the same position, but not be equal. I think I prefer the split of `IEqualityComparer`/`IComparer` (and the comparable `IEquatable`/`IComparable` for defining a default within a class) but what you describe would still do the trick most of the time. You had me in a panic, as I don't look at Java much but I was shocked at the idea it might not have any such equivalence provider, and that I might not have noticed! – Jon Hanna Oct 12 '10 at 23:02
  • @LukeH. You are entirely correct. Editing to replace with an order-agnostic method. – Jon Hanna Oct 12 '10 at 23:07
  • Suggestions: Why enumerate just the keys and do a `x[key]` lookup? Why not loop the dictionary itself? See Luke's answer. Also you would want to avoid boxing for value types. – nawfal Nov 08 '13 at 23:39
2

I would suggest using CollectionAssert.AreEquivalent() from NUnit. Assert.AreEqual() is really not meant for collections. http://www.nunit.org/index.php?p=collectionAssert&r=2.4

Alex Lo
  • 1,299
  • 8
  • 10
2

For later readers, here's what I've been told / been able to figure out:

  1. The contract for .NET collections, unlike Java collections, doesn't include any specific behavior for Equals() or GetHashCode().
  2. LINQ Enumerable.SequenceEqual() extension method will work for ordered collections, including dictionaries -- which present as IEnumerable<KeyValuePair>; KeyValuePair is a struct, and its Equals method uses reflection to compare the contents.
  3. Enumerable provides other extension methods that can be used to cobble together a content equality check, such as Union() and Intersect().

I'm coming around to the idea that, convenient as the Java methods are, they might not be the best idea if we're talking about mutable collections, and about the typical implicit equals() semantics -- that two equal objects are interchangeable. .NET doesn't provide very good support for immutable collections, but the open-source PowerCollections library does.

Community
  • 1
  • 1
David Moles
  • 48,006
  • 27
  • 136
  • 235
  • Java, unlike .NET, has no concept of an "equality comparer". When mutable types are used as entities, reference equality makes sense; when used as values which won't change while they're in a dictionary, value equality makes sense. In most cases where something that knows nothing about a class will be comparing instances, it's better to err on the side of reporting things unequal, which .NET's default reference equality does. In cases where code storing things in a dictionary knows that value equality makes more sense, it can specify an equality comparer that tests that. – supercat Nov 13 '13 at 20:14
  • Since Java doesn't have an equality comparer type, the only way types can make their values usable as dictionary keys is to override their equality-related methods to use value equality, and hope that instances which are being used as entities don't get stored into a dictionary. – supercat Nov 13 '13 at 20:16
1
public sealed class DictionaryComparer<TKey, TValue>
    : EqualityComparer<IDictionary<TKey, TValue>>
{
    public override bool Equals(
        IDictionary<TKey, TValue> x, IDictionary<TKey, TValue> y)
    {
        if (object.ReferenceEquals(x, y)) return true;
        if ((x == null) || (y == null)) return false;
        if (x.Count != y.Count) return false;

        foreach (KeyValuePair<TKey, TValue> kvp in x)
        {
            TValue yValue;
            if (!y.TryGetValue(kvp.Key, out yValue)) return false;
            if (!kvp.Value.Equals(yValue)) return false;
        }
        return true;
    }

    public override int GetHashCode(IDictionary<TKey, TValue> obj)
    {
        unchecked
        {
            int hash = 1299763;
            foreach (KeyValuePair<TKey, TValue> kvp in obj)
            {
                int keyHash = kvp.Key.GetHashCode();
                if (keyHash == 0) keyHash = 937;

                int valueHash = kvp.Value.GetHashCode();
                if (valueHash == 0) valueHash = 318907;

                hash += (keyHash * valueHash);
            }
            return hash;
        }
    }
}
LukeH
  • 263,068
  • 57
  • 365
  • 409
  • Minor suggestion: Use `EqualityComparer.Default.Equals(kvp.Value, yValue)` to avoid null exception in the `Equals` method. Similarly in `GetHashCode` method, you would need to check for `kvp.Value == null`. – nawfal Nov 08 '13 at 23:35
1

So it looks like key-value equality in C# is a property of the Dictionary concrete class, not of the IDictionary interface. Is this right? Is it generally true of the whole System.Collections framework?

If so, I'd be interested to read some discussion of why MS chose that approach

I think it is quite simple - IDictionary is an interface and interfaces can't have any implementations and in .NET world equality of two objects is defined through Equals method. So it is just impossible to override Equals for the IDictionary interface to allow it possesing "key-value equality".

Andrew Bezzub
  • 15,744
  • 7
  • 51
  • 73
0

you made a big mistake in your original post. You talked about the Equals() method in the IDictionary interface. That's the point!

Equals() is a virtual method of System.Object that classes can override. Interfaces don't implement methods at all. Instead, instances of interfaces are reference types, thus inheriting from System.Object and potentially declaring an override of Equals().

Now the point... System.Collections.Generic.Dictionary<K,V> does not override Equals. You said you implemented your IDictionary your own way, and reasonably overriden Equals, but look at your own code

Assert.AreEqual (backingDictionary, readOnlyDictionary); 

This method is basically implemented as return backingDictionary.Equals(readOnlyDictionary) and again here is the point.

Basic Equals() method returns false if two objects are instances of different classes, you cannot control that. Otherwise, if the two objects are of the same type, each member is compared via reflection (just members, not properties) using the Equals() approach instead of == (which is what the manual calls "value compare" instead of "reference compare")

So for first, I would not be surprised if Assert.AreEqual (readOnlyDictionary,backingDictionary); succeeds, because it would trigger a user-defined Equals method.

I have no doubts that approaches by other users in this thread work, but I just wanted to explain you what was the mistake in your original approach. Surely Microsoft would have better implemented an Equals method that compares the current instance to any other IDictionary instance, but, again, that would have gone outside the scope of the Dictionary class, which is a public stand-alone class and is not meant to be the only public available implementation of IDictionary. For example, when you define an interface, a factory and a protected class that implements it in a library, you might want to compare the class against other instances of the base interface rather than of the class itself which is not public.

I hope to have been of help to you. Cheers.

usr-local-ΕΨΗΕΛΩΝ
  • 26,101
  • 30
  • 154
  • 305
  • 1
    Sorry, I spoke imprecisely. It's the same situation in Java -- `equals()` is a method on `Object`. However, the Java `Map` interface "overrides" `equals()` (not really; it just re-declares it) to _document_ the _expectation_ that it return true for any two `Map` implementations with the same key->value mappings. It's what those expectations are in the .NET collections, and why, that I want to know. – David Moles Oct 25 '10 at 23:42
  • Got it now... you might need to read into .NET developers' minds :( – usr-local-ΕΨΗΕΛΩΝ Oct 26 '10 at 20:41
  • @djechelon: If two references are used to encapsulate identity, they should compare equal if and only if they identify the same object. Two unshared references which encapsulate state *and do not encapsulate identity* should compare equal if they encapsulate the same state. Unfortunately, neither .NET nor Java makes any distinction between references that should encapsulate identity and those that should not--a limitation which not complicates equality testing and cloning among other things, but is the underlying cuase of many mutability-related bugs. – supercat Nov 20 '13 at 23:07