Overriding Equals() but not checking all fields - what will happen?

Question

If I override Equals and GetHashCode, how do I decide which fields to compare? And what will happen if I have two objects with two fields each, but Equals only checks one field?

In other words, let's say I have this class:

class EqualsTestClass
{
    public string MyDescription { get; set; }
    public int MyId { get; set; }

    public override bool Equals(object obj)
    {
        EqualsTestClass eq = obj as EqualsTestClass;
        if(eq == null) {
            return false;
        } else {
            return MyId.Equals(eq.MyId);
        }
    }

    public override int GetHashCode()
    {
        int hashcode = 23;
        return (hashcode * 17) + MyId.GetHashCode();
    }
}

I consider two objects Equal if they have the same MyId. So if the Id is equal but the description is different, they are still considered equal.

I just wonder what the pitfalls of this approach are? Of course, a construct like this will behave as expected:

        List<EqualsTestClass> test = new List<EqualsTestClass>();

        EqualsTestClass eq1 = new EqualsTestClass();
        eq1.MyId = 1;
        eq1.MyDescription = "Des1";

        EqualsTestClass eq2 = new EqualsTestClass();
        eq2.MyId = 1;
        eq2.MyDescription = "Des2";

        test.Add(eq1);
        if (!test.Contains(eq2))
        {
            // Will not be executed, as test.Contains is true
            test.Add(eq2);
        }

As eq2 is value-equal to eq1, it will not be added. But that is code that I control, but I wonder if there is code in the framework that could cause unexpected problems?

So, should I always add all public Fields in my Equals() Comparison, or what are the guidelines to avoid a nasty surprise because of some bad Framework-Mojo that was completely unexpected?

Answer 1

The reason for overriding Equals() is that you define, what it means for two instances to be equal. In some cases that means that all fields must be equal, but it doesn't have to. You decide.

For more information see the documentation and this post.

Answer 2

I don't think you need to worry about the Framework in this instance. If you as the Class Designer consider two instances of that class to be equal if they share the same MyId, then, you only need to test MyId in your overriden Equals() and GetHashCode() methods.

Answer 3

You only need to check for the fields that are required to match, if all that needs to match is the ID then go with that.

Answer 4

A question: If I override Equals and GetHashCode, how do i decide which fields I compare?

It depends on what you are trying to accomplish. If you are trying to see if the objects are exactly the same you should compare all of them. If you have some 'key' and you only want to know if they are the same 'object', even if other data is different then just check the 'key' values.

And what will happen if I have two objects with two fields each, but Equals only checks one field?

Then you will have an equality method that just checks to see if the 'key' is the same, and could potentially have multiple 'equal' objects that have internal variances.

Answer 5

Others have said this is perfectly valid and expected, and it's exactly how Equals is supposed to operate. So there's no problem with it as a class.

I'd be very slightly wary of this as an API. Forgive me if it isn't what was intended: in that case this is just a note of caution to others.

The potential problem is that users of the API will naturally expect equal objects to "be the same". This isn't part of the contract of equality, but it is part of the common-sense meaning of the word. The class looks a bit like a binary tuple, but isn't one, so that should be for sensible reasons.

An example of such a sensible reason is that one field is a "visible implementation detail", like the max load factor on a hashtable-based container. An example of a risky (although tempting) reason is "because I added the description in afterwards and didn't want to change the Equals method in case it broke something".

So it's completely valid to do something a bit counter-intuitive, especially if you clearly document that the behaviour might be surprising. Such Equals methods have to be supported, because banning them would be crazy in cases where a field is obviously irrelevant. But it should be clear why it makes sense to create two ID-description pairs with the same ID and different descriptions, but it doesn't make sense to add them both to a container (like HashSet) which uses Equals/HashCode to prevent duplicate entries.

Answer 6

Your example is checking to see if a list(of EqualsTestClass) contains an object of the same type with the same property values. Another way to accomplish this task without overriding equals (and the traditional understanding of equals) is to use a custom comparer. It would look something like this (in VB):

Public Class EqualsTestComparer
Implements IEqualityComparer(Of EqualsTestClass)
Public Function Equals1(ByVal x As EqualsTestClass, ByVal y As EqualsTestClass) As Boolean Implements System.Collections.Generic.IEqualityComparer(Of EqualsTestClass).Equals
    If x.MyId  = y.MyId and x.MyDescription = y.MyDescription  Then
        Return True
    Else
        Return False
    End If
End Function

Public Function GetHashCode1(ByVal obj As EqualsTestClass) As Integer Implements System.Collections.Generic.IEqualityComparer(Of EqualsTestClass).GetHashCode
    Return obj.ToString.ToLower.GetHashCode
End Function    
End Class

Then in your routine you simply use the custom comparer:

If Not test.Contains(eq2, New EqualsTestComparer) Then
    //Do Stuff
End if