Clinton
Clinton

Reputation: 23135

Implementing value equality for structs

I've read a number of things that leaving the default Equals() and GetHashCode() method for structs is slow at best and even incorrect in cases where a struct has different bitwise representations which are equal.

So I've written the following ValueEquality class:

public abstract class ValueEquality<T> : IEquatable<T> 
    where T : ValueEquality<T>
{
    public override bool Equals(object obj)
    {
        return Equals(obj as T);
    }

    public static bool operator ==(ValueEquality<T> lhs, T rhs)
    {
        return ReferenceEquals(lhs, null) ? ReferenceEquals(rhs, null) : lhs.Equals(rhs);
    }

    public static bool operator !=(ValueEquality<T> lhs, T rhs)
    {
        return !(lhs == rhs);
    }

    public bool Equals(T other)
    {
        return ReferenceEquals(this, other) || ((!ReferenceEquals(other, null)) && EqualsNoNull(other));
    }

    public bool EqualsNoNull(T other)
    {
        IEnumerable<object> thisMembers = GetMembers();
        IEnumerable<object> otherMembers = other.GetMembers();
        return thisMembers.ObjectEquals(otherMembers);
    }

    public override int GetHashCode()
    {
        unchecked
        {
            int hash = 17;
            foreach (object member in GetMembers())
            {
                hash = hash * 31 + member.GetHashCode();
            }
            return hash;
        }
    }

    protected abstract IEnumerable<object> GetMembers();
}

The ObjectEquals() extension method on IEnumerable simply returns true if all the corresponding Equals() calls on the respective enumerations return true (and they're the same length, of course).

Here's an example class implementing this:

class C : ValueEquality<C>
{
    public readonly int I;
    public readonly double D;

    protected override IEnumerable<object> GetMembers()
    {
        yield return I;
        yield return D;
    }
}

This seems to work quite well. But there's two issues:

  1. This only works with classes.
  2. It only works with classes that don't already have a subclass.

If I made this an interface it kind off defeats the purpose, I'll have to reimplement each method. I guess I could put these implememtations in extension methods but it's still a lot of boilerplate, whereas now I only have to implement one method.

So I've got two questions:

  1. Is there a way to achieve what I'm doing without all the boilerplate for each struct I want to implement it for?
  2. If the answer is "No" to 1, will default interface methods in C#8 help here?

Upvotes: 0

Views: 97

Answers (1)

madreflection
madreflection

Reputation: 4947

  1. Is there a way to achieve what I'm doing without all the boilerplate for each struct I want to implement it for?

1. Not really, no.

You pointed out that it has some issues, such as only working for classes. It's worse than that, though, and you hinted at it in the following point. It locks your type into having a base class that it will have in common with other classes simply because they share that particular implementation detail. There's already a type like that: System.Object. So with this, of all the types that have value equality, a subset leak that implementation detail through their class hierarchy. That seems like a pretty major code smell.

I believe another drawback is -- and I'm no expert on this -- that hash code calculation isn't one-size-fits-all. If I'm right about that, you'll need to be able to choose the right combining algorithm for each class.

  1. If the answer is "No" to 1, will default interface methods in C#8 help here?

2. Absolutely not.

For one thing, in order to provide a default implementation for an interface method, you have to have control of the interface. Since you have control of neither Object (also, not even an interface) nor IEquatable<T>, you can't create a default implementation for their methods.

Furthermore, Object already has a default implementation. It's what you're trying to avoid. IEquatable<T> could be default-implemented by the BCL in terms of Object.Equals but that's backwards and, again, what you're trying to avoid. Side effects include boxing of value types, calling Equals(Object) even if it's not overridden, and mild developer disorientation.

And finally, introducing a new interface would have no effect because, 1) the default implementation you're able to provide would be for a method in that interface, not one in Object or IEquatable<T>, and 2) nothing in the Framework would be meaningfully aware of the new interface so it would never be meaningfully called (i.e. for equality testing).

Where to go from here

It may not look good for what you were trying to do but I don't want to leave you empty-handed.

If you want a reusable implementation, I suggest creating a utility class that provides the functionality as static methods. It does involve some boilerplate code in each type, but it's more uniform than just writing it out and the effort of writing it can be minimized using code generation or snippets (I lean toward the latter for something this simple, myself).

Because it doesn't require inheritance, you can use it for value types, as well, and the code is almost identical.

// Signatures for up to 4 fields.

public static class EqualityUtility
{
    public static int GetHashCode<T1>(T prop1);
    public static int GetHashCode<T1, T2>(T prop1, T2 prop2);
    public static int GetHashCode<T1, T2, T3>(T prop1, T2 prop2, T3 prop3);
    public static int GetHashCode<T1, T2, T3, T4>(T prop1, T2 prop2, T3 prop3, T4 prop4);

    public static bool OtherIsNotNull<T>(T other) where T : class => !(other is null);

    public static bool Equals<T1>(T1 obj1prop1, T1 obj2prop1);
    public static bool Equals<T1, T2>(T1 obj1prop1, T1 obj2prop1, T2 obj1prop2, T2 obj2prop2);
    public static bool Equals<T1, T2, T3>(T1 obj1prop1, T1 obj2prop1, T2 obj1prop2, T2 obj2prop2, T3 obj1prop3, T3 obj2prop3);
    public static bool Equals<T1, T2, T3, T4>(T1 obj1prop1, T1 obj2prop1, T2 obj1prop2, T2 obj2prop2, T3 obj1prop3, T3 obj2prop3, T4 obj1prop4, T4 obj2prop4);
}

No, it's not exactly pretty. Yes, it gets unwieldy rather quickly.

// Demonstrations with 3 fields:

public struct TestStruct : IEquatable<TestStruct>
{
    private int _field1;
    private string _field2;
    private double _field3;

    // Concerns like constructors excluded for brevity...

    public override int GetHashCode() => EqualityUtility.GetHashCode(_field1, _field2, _field3);

    public override bool Equals(object obj) => obj is TestStruct other && Equals(other);

    public bool Equals(TestStruct other) =>
        EqualityUtility.Equals(_field1, other.field1, _field2, other.field2, _field3, other._field3);

    // Everything else would be implemented in terms of what you already have.
    public static bool operator ==(TestStruct a, TestStruct b) => a.Equals(b);
    public static bool operator !=(TestStruct a, TestStruct b) => !a.Equals(b);
}

public class TestClass : IEquatable<TestClass>
{
    private int _field1;
    private string _field2;
    private double _field3;

    // Concerns like constructors excluded for brevity...

    public override int GetHashCode() => EqualityUtility.GetHashCode(_field1, _field2, _field3);

    public override bool Equals(object obj) => obj is TestClass other && Equals(other);

    public bool Equals(TestClass other) =>
        EqualityUtility.OtherIsNotNull(other) &&
        EqualityUtility.Equals(_field1, other.field1, _field2, other.field2, _field3, other._field3);

    public static bool operator ==(TestClass a, TestClass b) => (a is null && b is null) || (!(a is null) && a.Equals(b));
    public static bool operator !=(TestClass a, TestClass b) => !(a == b);
}

The utility class could be provided as a (private?) NuGet package, with either an assembly that gets referenced or a code file that gets compiled with the project (in which case you should make the class internal).

Upvotes: 1

Related Questions