Overloading operator== versus Equals()

Question

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 ==?

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 always check for reference equality (the main exception being when comparing nullable types to `Nothing`). — supercat, Jun 25 '12 at 22:07

score 97 · Answer 1 · edited Jun 19 '16 at 18:51

97

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

edited Jun 19 '16 at 18:51

John Weisz

30,137
13
89
132

answered Dec 04 '09 at 19:56

Samuel Neff

73,278
17
138
182

1

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.) – JSBձոգչ Dec 04 '09 at 20:50
1

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. – drdwilcox Nov 18 '11 at 00:12
1

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? – Lee Louviere Nov 18 '11 at 19:07
@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. – Samuel Neff Nov 20 '11 at 21:29
@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... – supercat Sep 24 '12 at 23:00
...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`. – supercat Sep 24 '12 at 23:02
4

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. – David Arno May 13 '13 at 12:26
@drdwilcox 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. – ToolmakerSteve Sep 27 '13 at 01:30
@supercat 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). – ToolmakerSteve Sep 27 '13 at 01:39
@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". – supercat Sep 27 '13 at 14:52
@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. – supercat Sep 27 '13 at 15:00
@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. – supercat Sep 27 '13 at 15:10

score 27 · Accepted Answer · edited Mar 12 '13 at 13:22

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.

edited Mar 12 '13 at 13:22

Liam

27,717
28
128
190

answered Nov 19 '09 at 21:07

McKay

12,334
7
53
76

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. – H H Nov 19 '09 at 22:10
3

Also, be careful of nulls here. x.Equals(null) will throw an NRE if x is null, whereas null == x will work. – darthtrevino Nov 19 '09 at 22:58
1

'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. – Bill Yang Nov 19 '09 at 23:06
3

Bill, but it is a very good standard. Would you like a type where `a != b` but the Collection classes think they are duplicates? – H H Nov 21 '09 at 10:26
@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). – Lee Louviere Nov 28 '11 at 14:24

score 8 · Answer 3 · edited Sep 16 '16 at 19:17

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();
    }
}

edited Sep 16 '16 at 19:17

Chef_Code

241
4
11

answered Feb 09 '13 at 23:45

John

633
4
9

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. – Servy Feb 10 '13 at 00:49
3

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. – friederbluemle Feb 14 '13 at 05:01
1

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. http://msdn.microsoft.com/en-us/library/336aedhh(v=vs.85).aspx Observe "GetType() != obj.GetType())". This is essential when subclasses exist. – ToolmakerSteve Sep 27 '13 at 02:21
@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. – Ben Voigt Jul 14 '14 at 03:11

H H · Answer 4 · 2009-11-19T22:18:13.940

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.

edited Nov 19 '09 at 22:18

answered Nov 19 '09 at 22:13

H H

263,252
30
330
514

I'm comparing an attribute to a methodinfo. Those are semantically immutable. – Lee Louviere Nov 28 '11 at 14:22

score 7 · Answer 5 · edited Mar 12 '13 at 13:23

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

edited Mar 12 '13 at 13:23

Liam

27,717
28
128
190

answered Nov 19 '09 at 21:15

Mike Sackton

1,094
7
19

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). – ToolmakerSteve Sep 27 '13 at 01:33

score 6 · Answer 6 · edited Sep 16 '16 at 19:47

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

edited Sep 16 '16 at 19:47

Chef_Code

241
4
11

answered Sep 21 '12 at 08:32

Ykok

1,313
13
15

6

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. – supercat Sep 24 '12 at 22:49

Overloading operator== versus Equals()

6 Answers6

Linked