2

Please see the code below:

public virtual bool Equals(Entity other)
        {
            return Equals((Object)other);
        }

public override bool Equals(object obj)
        {
            var compareTo = obj as Entity;
            if (ReferenceEquals(compareTo, null))
                return false;
            if (ReferenceEquals(this, compareTo))
                return true;
            if (GetType() != compareTo.GetType())
                return false;
            if (!IsTransient() && !compareTo.IsTransient() && Id == compareTo.Id)
                return true;
            return false;
        }

IEquatable.Equals and Object.Equals override should return the same result. Therefore if there is a call to IEquatable.Equals then it just calls the override for Object.Equals.

Is this a standard approach? I am trying to follow the principle of least astonishment. Alternatively I could just duplicate the code in both methods.

10
  • 1
    "Is this a standard approach?" - define standard. But it seems to be a simple and working approach. "Alternatively I could just duplicate the code in both methods" - definitely no. Already forgot the DRY principle I mentioned in an answer to that other question of yours 6 hours ago?
    – Doc Brown
    Jan 23, 2018 at 18:30
  • 3
    Most of the time the Equals(object) override defers to the strongly-typed method, but it's a wash either way. Jan 23, 2018 at 18:33
  • That direction involves more casting and type checking (which has readability issues as well as runtime costs), compared to the other direction shown by @mmathis.
    – Erik Eidt
    Jan 23, 2018 at 19:21
  • 1
    Note that it only gets worse if you want to also apply a total order via IComparable<T> or < and > operators. What I do in that case is make one private method that takes (T t1, T t2), and returns -1, 0 or 1, and then every public facing comparison method calls it. See ericlippert.com/2013/10/07/… for an example. Jan 26, 2018 at 20:48
  • 1
    @Abel: Designing types that have non-standard comparisons, including values that are not equal to themselves, and so on, is for experts only. If you're such an expert then you already know how to do it correctly and safely without my advice! The vast majority of C# programmers are not designing such types, and are not experts on comparison logic; those developers should learn the rules and carefully follow them. The techniques I'm describing are to help programmers new to these concepts be successful in writing correct, robust code the first time. Jan 7, 2019 at 18:32

1 Answer 1

9

The more standard approach I have seen is with the methods defined the other way around:

public virtual bool Equals(Entity other)
    {
        return other != null && ID == other.ID;
    }

public override bool Equals(object obj)
    {
        return Equals(obj as Entity);
    }

This way you don't have to worry about checking that the types match, as that's taken care of already. As DocBrown mentioned in the comments, though, the important thing is that you don't duplicate the code between methods - have one method call the other. If your implementation above is working for you, there's not much reason to change it.

3
  • Forgive my ignorance of C# but, doesn't obj as Entity perform a cast? Won't that throw an exception rather than return the (desired) false? Jan 24, 2018 at 5:56
  • 2
    @Maybe_Factor, no, as performs a type check and returns null if the check fails (including if the value is null), otherwise it performs the cast. Roughly speaking.
    – David Arno
    Jan 24, 2018 at 8:20
  • @Maybe_Factor As mentioned by @DavidArno, as is a "safe" cast, in that it will never throw an exception. If the object is not of the type, it returns null. (Entity)object will throw an exception if object is not of type Entity
    – mmathis
    Jan 26, 2018 at 15:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.