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.

  • 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 '18 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. – Mark Benningfield Jan 23 '18 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 '18 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. – Eric Lippert Jan 26 '18 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. – Eric Lippert Jan 7 at 18:32
8

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.

  • 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? – Maybe_Factor Jan 24 '18 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 '18 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 '18 at 15:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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