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

Improve this question
1
  • 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). Commented Jun 25, 2012 at 22:07

6 Answers 6

Reset to default
96

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
Improve this answer
Sign up to request clarification or add additional context in comments.

12 Comments

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.)
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.
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?
@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.
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.
|
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.

Improve this answer

5 Comments

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.
Also, be careful of nulls here. x.Equals(null) will throw an NRE if x is null, whereas null == x will work.
'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, but it is a very good standard. Would you like a type where a != b but the Collection classes think they are duplicates?
@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).
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(); } }
Improve this answer

4 Comments

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.
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.
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.
@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.
7

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.

Improve this answer

1 Comment

I'm comparing an attribute to a methodinfo. Those are semantically immutable.
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

Improve this answer

1 Comment

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).
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

Improve this answer

1 Comment

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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.