10

What's your approach on writing equality checks for the structs and classes you create?

1) Does the "full" equality checking require that much of boilerplate code (like override Equals, override GetHashCode, generic Equals, operator==, operator!=)?

2) Do you specify explicitly that your classes model the IEquatable<T> interface?

3) Do I understand correctly, that there is no actual way to automatically apply Equals overrides, when I invoke something like a == b and I always have to implement both the Equals and operator== members?

Yippie-Ki-Yay
  • 22,026
  • 26
  • 90
  • 148

4 Answers4

20

You're right, this is a lot of boiler-plate code, and you need to implement everything separately.

I would recommend:

  • If you're going to implement value equality at all, override GetHashCode and Equals(object) - creating overloads for == and implementing IEquatable<T> without doing that could result in very unexpected behaviour
  • I would always implement IEquatable<T> if you're overriding Equals(object) and GetHashCode
  • I only overload the == operator more rarely
  • Implementing equality correctly for unsealed classes is tricky, and can still produce surprising/undesirable results. If you need equality for types in a hierarchy, implement IEqualityComparer<T> expressing the comparison you're interested in.
  • Equality for mutable types is usually a bad idea, as two objects can be equal and then unequal later... if an object is mutated (in an equality-affecting way) after it's been used as a key in a hash table, you won't be able to find it again.
  • Some of the boiler-plate is slightly different for structs... but like Marc, I very rarely write my own structs.

Here's a sample implementation:

using System;

public sealed class Foo : IEquatable<Foo>
{
    private readonly string name;
    public string Name { get { return name; } }

    private readonly int value;
    public int Value { get { return value; } }

    public Foo(string name, int value)
    {
        this.name = name;
        this.value = value;
    }

    public override bool Equals(object other)
    {
        return Equals(other as Foo);
    }

    public override int GetHashCode()
    {
        int hash = 17;
        hash = hash * 31 + (name == null ? 0 : name.GetHashCode());
        hash = hash * 31 + value;
        return hash;
    }

    public bool Equals(Foo other)
    {
        if ((object) other == null)
        {
            return false;
        }
        return name == other.name && value == other.value;
    }

    public static bool operator ==(Foo left, Foo right)
    {
        return object.Equals(left, right);
    }

    public static bool operator !=(Foo left, Foo right)
    {
        return !(left == right);
    }
}

And yes, that's a heck of a lot of boilerplate, very little of which changes between implementations :(

The implementation of == is slightly less efficient than it might be, as it will call through to Equals(object) which needs to do the dynamic type check... but the alternative is even more boiler-plate, like this:

public static bool operator ==(Foo left, Foo right)
{
    if ((object) left == (object) right)
    {
        return true;
    }

    // "right" being null is covered in left.Equals(right)
    if ((object) left == null)
    {
        return false;
    }
    return left.Equals(right);
}
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 2 minor suggestions for the second code block: 1) Shouldn't you move `(object) left == (object) right` from `==` to generic `Equals`? So that gives the speed (of course it depends, but assuming the worst case) checking of reference equality even for generic `Equals` method? 2) you dont need the second null check `(object) right == null` in `==` as you essentially do that in generic `Equals`. See my post.. – nawfal Dec 16 '12 at 20:53
  • @nawfal: I don't think there's much point in doing it in the generic `Equals` case - it's going to be quick anyway in the cases where it *is* true, and for the cases where it *isn't* true, it's adding an extra check for no benefit. As for the null part - that would require the dynamic type check again. Yes, you could argue for both - but I'm happy enough with what I wrote two years ago... – Jon Skeet Dec 16 '12 at 20:58
  • @nawfal: Um, you're right - there'd be no dynamic check there. I'll remove the "right" part here... – Jon Skeet Dec 16 '12 at 21:09
  • @JonSkeet its so frustrating to write this boilerplate code over and over when making a mistake is so easy in these things. Thankfully snippets can help in this regard! I would have tried to get away by writing a generic abstract base class that does the job (I know the potential dangers, but if I can manage, then thats ok) to be derived by base classes, but that limits the derived classes from inheriting from another base class.. Just saying.. – nawfal Dec 16 '12 at 21:14
6

I rarely do anything special for classes; for most regular objects referential equality works fine.

I even more rarely write a struct; but since structs represent values it is usually appropriate to provide equality etc. This would usually involve everything; Equals, ==, != and IEquatable<T> (since this avoids boxing in scenarios using EqualityComparer<T>.Default.

The boilerplate isn't usually too problematic, but IIRC tools like resharper can help here.

Yes, it is advisable to keep Equals and == in sync, and this needs to be done explicitely.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
1

You just need to implement operator== for a==b .
As I like my data in dictionaries sometimes I override GetHashCode.
Next I implement Equals (as an unmentioned standard ... this is because there is no constraint for equality when using generics) and specify implementing IEquatable. Since I am going to do this I might as well point my == and != implementations to Equals. :)

basarat
  • 261,912
  • 58
  • 460
  • 511
0

See What is "Best Practice" For Comparing Two Instances of a Reference Type?

You can avoid boiler plate code (hope C#/VS team brings something easy for developers in their next iteration) with the help of a snippet, here is one such..

Community
  • 1
  • 1
nawfal
  • 70,104
  • 56
  • 326
  • 368