Overloading operator== versus Equals()
Asked Answered
O

6

48

I'm working on a C# project on which, until now, I've used immutable objects and factories to ensure that objects of type Foo can always be compared for equality with ==.

Foo objects can't be changed once created, and the factory always returns the same object for a given set of arguments. This works great, and throughout the code base we assume that == always works for checking equality.

Now I need to add some functionality that introduces an edge case for which this won't always work. The easiest thing to do is to overload operator == for that type, so that none of the other code in the project needs to change. But this strikes me as a code smell: overloading operator == and not Equals just seems weird, and I'm used to the convention that == checks reference equality, and Equals checks object equality (or whatever the term is).

Is this a legitimate concern, or should I just go ahead and overload operator ==?

Onagraceous answered 19/11, 2009 at 21:0 Comment(1)
Incidentally, vb.net forbids the use of its = and <> equality operators for types which do not provide explicit overloads; to check for reference equality, one uses Is or IsNot, which essentially <i>always</i> check for reference equality (the main exception being when comparing nullable types to Nothing).Motorboating
D
27

I believe the standard is that for most types, .Equals checks object similarity, and operator == checks reference equality.

I believe best practice is that for immutable types, operator == should be checking for similarity, as well as .Equals. And if you want to know if they really are the same object, use .ReferenceEquals. See the C# String class for an example of this.

Diseuse answered 19/11, 2009 at 21:7 Comment(5)
No. The 'standard' for reference types is for both Equals en == to return the same as RefernceEquals. It is discouraged to overload either for non-immutable types.Sixpack
Also, be careful of nulls here. x.Equals(null) will throw an NRE if x is null, whereas null == x will work.Spiritual
'standard' is a common way to solve certain type of problem, if the 'standard' gets in the way, then it's not very good standard.Novation
Bill, but it is a very good standard. Would you like a type where a != b but the Collection classes think they are duplicates?Sixpack
@Bill, I just got bit by not following the standard. Which is why I'm asking what it is right here. That should be sign enough that the standard is important. There's a difference between coding style standards (Which are to promote readability within a group) and practical coding standards (Which avoid a lot of headache).Therm
C
98

There's a big difference between overloading == and overriding Equals.

When you have the expression

if (x == y) {

The method that will be used to compare variables x and y is decided at compile time. This is operator overloading. The type used when declaring x and y is used to define which method is used to compare them. The actual type within x and y (i.e., a subclass or interface implementation) is irrelevant. Consider the following.

object x = "hello";
object y = 'h' + "ello"; // ensure it's a different reference

if (x == y) { // evaluates to FALSE

and the following

string x = "hello";
string y = 'h' + "ello"; // ensure it's a different reference

if (x == y) { // evaluates to TRUE

This demonstrates that the type used to declare the variables x and y is used to determine which method is used to evaluate ==.

By comparison, Equals is determined at runtime based on the actual type within the variable x. Equals is a virtual method on Object that other types can, and do, override. Therefore the following two examples both evaluate to true.

object x = "hello";
object y = 'h' + "ello"; // ensure it's a different reference

if (x.Equals(y)) { // evaluates to TRUE

and the following

string x = "hello";
string y = 'h' + "ello"; // ensure it's a different reference

if (x.Equals(y)) { // also evaluates to TRUE
Coumarin answered 4/12, 2009 at 19:56 Comment(12)
This is a very important point, and it's something I came across later while I was testing out this. Since I needed operator== to depend on the run-time type, I defined an operator overload in the base class that called Equals(). Worked like a charm. (Then I redid the object model and tore the whole thing out. But that's a different story.)Temptation
I totally disagree. The use of == is a common idiom. Just because C# bases all entities to object doesn't mean that we should abandon the idiom to prevent the situation in this post. Rather, it becomes good practice to use .Equals() when needing a comparison at the top of the hierarchy.Conk
In short, overload == to set which method to call (Object.Equals, or Object.ReferenceEqual). Overload Object.Equals to check for actual value equality. Am I right?Therm
@Xaade, Having different behavior for '' and Equals is very dangerous IMO. They should be programmed to do the same thing and you can provide various implementations of IEqualityComparer to offer alternative equal implementations.Coumarin
@SamuelNeff: To my mind, X.Equals(Y)` when X and Y do not behave semantically identically is a code smell far worse than having X.Equals(Y) return a different result from X==Y. Among other things, the fact that type coercion can occur before operands are compared with == means that X==Y cannot very well define a recurrence relation. For example, 123456789123456789 == 123456789123456789.0 yields true, as does 123456789123456788 == 123456789123456789.0, but 123456789123456789 == 123456789123456788 yields false. The idea that Equals(Object) and == should behave the same...Motorboating
...has unfortunately resulted in some bad overrides of Equals(Object) in Decimal, Single, and Double. To my mind, the equality-test operator should check whether two parameters are identical in the way most commonly associated with the type, while the Equals method should test for stronger equivalence. I would consider 1.0m to be equal to, but not equivalent to, 1.00m.Motorboating
Wow, that is one scary cock-up in the design of .NET. The idea that two strings won't be equal just because I used object x, rather than string x, to define my variable is a prize-grade code smell. Thanks for explaining this so well.Robbert
@Conk IMHO "a == b" would be useful if it were merely syntactic sugar for calling Object.Equal(a, b). As it is, it can lead to subtle bugs. I believe it is a fundamental mistake on Microsoft's part to have "=="s behavior depend on the compile-time declarations of variables. I just ran into a case where a programmer defined "==" (for convenience), and then a subclass was added. Of course, the subclass's "==" didn't get called since the calling code was referring to the base class. Microsoft has made "==" unusable, except in limited situations.Ellora
@Motorboating What is a situation where "==" should do something other than call .Equals? I've seen various suggestions that "==" should call ReferenceEquals, but that seems quite strange to me. If one wants IDENTITY test, then use .Net's identity test (ReferenceEquals).Ellora
@ToolmakerSteve: There's not usually much point overloading == to simply call ReferenceEquals, since that's what the == operator will test anyway. As for when it should differ from Equals, I gave an example; perhaps I need to provide some more background. When working with immutable classes, one will frequently create many identical objects--not deliberately, but often because one happens to perform the same operations on the same operands. For example, every time one calls X.ToString() when X happens to be 34, it will create a new two-character string containing the characters "34".Motorboating
@ToolmakerSteve: If one is constructing many immutable objects and expects that many independently-created instances will in fact be identical, it may be helpful to use a "weak interning dictionary" so that if one finds that a newly-created instance matches one that already exists, one can abandon the new instance and use a reference to the old one instead. In some situations this can cut memory usage by an order of magnitude (big savings), but for this to be safe, it's imperative that the new object be an exact replacement for the old one.Motorboating
@ToolmakerSteve: I would posit that immutable types should define Equals such that if X.Equals(Y) will only return true if the class is would expect that unless external code performs reference-equality-based operations, one could safely replace some or all occurrences of X with Y without any effect other than possible improved performance. By that definition, 1.00m.Equals(1.0m), since replacing 1.00m with 1.0m could have visible effects other than performance.Motorboating
D
27

I believe the standard is that for most types, .Equals checks object similarity, and operator == checks reference equality.

I believe best practice is that for immutable types, operator == should be checking for similarity, as well as .Equals. And if you want to know if they really are the same object, use .ReferenceEquals. See the C# String class for an example of this.

Diseuse answered 19/11, 2009 at 21:7 Comment(5)
No. The 'standard' for reference types is for both Equals en == to return the same as RefernceEquals. It is discouraged to overload either for non-immutable types.Sixpack
Also, be careful of nulls here. x.Equals(null) will throw an NRE if x is null, whereas null == x will work.Spiritual
'standard' is a common way to solve certain type of problem, if the 'standard' gets in the way, then it's not very good standard.Novation
Bill, but it is a very good standard. Would you like a type where a != b but the Collection classes think they are duplicates?Sixpack
@Bill, I just got bit by not following the standard. Which is why I'm asking what it is right here. That should be sign enough that the standard is important. There's a difference between coding style standards (Which are to promote readability within a group) and practical coding standards (Which avoid a lot of headache).Therm
S
8

It definitely smells. When overloading == you should make sure that both Equals() and GetHashCode() are also consistent. See the MSDN guidelines.

And the only reason that this seems OK at all is that you describe your type as immutable.

Sixpack answered 19/11, 2009 at 22:13 Comment(1)
I'm comparing an attribute to a methodinfo. Those are semantically immutable.Therm
E
8

Sample showing how to implement this according to MSFT guidelines (below). Notice, when overriding Equals you also need to override GetHashCode(). Hope this helps folks.

public class Person
{
    public Guid Id { get; private set; }

    public Person(Guid id)
    {
        Id = id;
    }

    public Person()
    {
        Id = System.Guid.NewGuid();
    }

    public static bool operator ==(Person p1, Person p2)
    {
        bool rc;

        if (System.Object.ReferenceEquals(p1, p2))
        {
            rc = true;
        }
        else if (((object)p1 == null) || ((object)p2 == null))
        {
            rc = false;
        }
        else
        {
            rc = (p1.Id.CompareTo(p2.Id) == 0);
        }

        return rc;
    }

    public static bool operator !=(Person p1, Person p2)
    {
        return !(p1 == p2);
    }

    public override bool Equals(object obj)
    {
        bool rc = false;
        if (obj is Person)
        {
            Person p2 = obj as Person;
            rc = (this == p2);
        }
        return rc;
    }

    public override int GetHashCode()
    {
        return Id.GetHashCode();
    }
}
Exemption answered 9/2, 2013 at 23:45 Comment(4)
Your null check is flawed. If both of the people are null it will return false when it should return true. You want to use an exclusive OR between the two null checks instead of a boolean OR.Malloch
ReferenceEquals will return true if both p1 and p2 are null. Therefore the boolean OR code will never execute and the operator will return true as expected.Oxyacid
BTW, Your coding style won't work if Person can be subclassed. Instead, put the logic in the "Equals" method, and have operator "==" call Object.Equals(a, b). REASON: which "==" gets called is based on the COMPILE-TIME type rather than the DYNAMIC type. Object.Equals does dispatch dynamically. If Person is subclassed, it is also necessary to EXACTLY compare types, not merely cast to Person. An example of correct coding of "Equals" is here. msdn.microsoft.com/en-us/library/336aedhh(v=vs.85).aspx Observe "GetType() != obj.GetType())". This is essential when subclasses exist.Ellora
@ToolmakerSteve: But making == act differently from all other operators isn't that great a design either... Care is needed when combining operators with polymorphic references.Doolie
C
7

For immutable types I don't think there is anything wrong with having == overloaded to support value equality. I don't think I'd override == without overriding Equals to have the same semantics however. If you do override == and need to check reference equality for some reason, you can use Object.ReferenceEquals(a,b).

See this Microsoft article for some useful guidelines

Collen answered 19/11, 2009 at 21:15 Comment(1)
Only a good idea if the immutable type is also SEALED. If the type can be subclassed, it is most regrettable that WHICH "==" gets called is determined by the COMPILE-TIME variable declaration, NOT by the dynamic type at run-time (which might be the subclass).Ellora
M
6

According to Microsofts own best practises the outcome of the Equals method and the equals (==) overload should be the same.

CA2224: Override equals on overloading operator equals

Manolete answered 21/9, 2012 at 8:32 Comment(1)
In nearly all circumstances where one overloads the statically-bound == one should also override the virtual Equals member, but the reverse is not true. One should only overload == for sealed types which behave very much like values (the way String does), but one should often override Equals for unsealed types. Note that a lot of programmers expect that applying the == operator to any class type other than string is a shorthand for checking reference-equality.Motorboating

© 2022 - 2024 — McMap. All rights reserved.