28

For the code below

public struct Person
{
    public int ID;
    public static bool operator ==(Person a, Person b) { return  a.Equals(b); }
    public static bool operator !=(Person a, Person b) { return !a.Equals(b); }
}

Why does the compiler give me these warnings?
What's wrong with not defining the methods below?

warning CS0660: 'Person' defines operator == or operator != but
    does not override Object.Equals(object o)

warning CS0661: 'Person' defines operator == or operator != but
    does not override Object.GetHashCode()
user541686
  • 205,094
  • 128
  • 528
  • 886
  • I think you're right. The `==` and `!=` operators would not be there (it's a `struct`!) without you defining them. On the other hand you're definig them to be exactly eqivalent to the behaviour of `ValueType.Equals(Object)`, obviously. So it would look strange to override that method without changing it. The compiler does not, however, realise that (nor check if) the body of your `==` implementation is entirely equivivalent to `Equals`, I suppose. – Jeppe Stig Nielsen May 28 '12 at 21:43
  • @JeppeStigNielsen: Yeah that's what I thought at first too, but then I thought: the problem would still exist even if I *did* override `Equals` (i.e. the compiler couldn't verify the body of `==`), so that can't be the reason... – user541686 May 28 '12 at 21:53
  • 4
    Warnings are not generated by a very intelligent being. You know, I see people overriding (and changing) `Equals` without overriding `GetHashCode`. The compiler warns them. Good! Then they type into VS: `override Ge` and they see a completion that they choose. The editor has written for them: `public overide int GetHashCode() { return base.GetHashCode(); }` The compiler no longer warns :-( They go ahead and ship the code ... – Jeppe Stig Nielsen May 28 '12 at 22:16

8 Answers8

23

EDIT: This answer has been corrected, among other things to note that user-defined value types don't generate ==, and to mention the performance issues with ValueType.Equals.


In general, overridding one, but not all, is confusing. The user expects neither to be overridden, or both to be, with the same semantics.

Microsoft's recommendations for this state (among other things):

  • Implement the GetHashCode method whenever you implement the Equals method. This keeps Equals and GetHashCode synchronized.

  • Override the Equals method whenever you implement the equality operator (==), and make them do the same thing.

In your case, you have a valid reason to defer to Equals (the compiler doesn't automatically implement ==) and override only those two (==/!=). However, there's still a performance issue, since ValueType.Equals uses reflection:

"Override the Equals method for a particular type to improve the performance of the method and more closely represent the concept of equality for the type."

Thus, it's still recommended to override all (==/!=/Equals) in the end. Of course, performance may not matter for this trivial struct.

Matthew Flaschen
  • 278,309
  • 50
  • 514
  • 539
  • 1
    Sorry I don't understand... how are the semantics any different when I don't override them? – user541686 May 28 '12 at 21:06
  • @Mehrdad, you're right. In this case, you shouldn't override anything equality-related, since you want the default. – Matthew Flaschen May 28 '12 at 21:09
  • *"The default == and != are already what you want."* -- the trouble is, that's not always true. E.g. operator `==` is *not* predefined if the field is, say, a `string`, instead of an `int`. And yet, I don't see anything wrong with my method in that case, either. Or is there? – user541686 May 28 '12 at 21:16
  • 1
    But in this case the Original Poster already did ensure that `Equals(Object)` and `==` "do the same thing" (your quote). – Jeppe Stig Nielsen May 28 '12 at 21:46
  • @Mehrdad, I was wrong. User-defined value types do not have an automatic `==` (regardless of the fields). – Matthew Flaschen May 28 '12 at 21:54
  • @MatthewFlaschen: Haha I guess my testing was wrong too, since I made the same mistake... thanks for pointing it out. – user541686 May 28 '12 at 21:57
  • @Mehrdad, one more thing. You don't have to override `Equals`. However, it's still recommended, for performance reasons. – Matthew Flaschen May 28 '12 at 22:14
  • @MatthewFlaschen: Yup I'm aware of that. :) – user541686 May 28 '12 at 22:20
  • The compiler doesn't care (in the sense that it's not going to bother checking) if your `==` and `Equals` implementations match, it just cares that you didn't provide one. In its mind, that's almost certainly a bug, so it wants you to fix it (valid performance concerns aside). – Michael Edenfield May 28 '12 at 22:22
7

There is a general expectation within the Framework that certain operations should always produce the same result. The reason is that certain operations (in particular, sorting and searching, which make up a large portion of any application) rely on these different operations producing meaningful and consistent results. In this case, you are breaking a couple of those assumptions:

  • If there is a valid operation == between a and b, it should produce the same result as a.Equals(b)
  • Similar, if there is a valid operation != between a and b, it should produce the same result as !a.Equals(b)
  • If two objects a and b exist, for which a == b, then a and b should produce the same key when stored in a hash table.

The first two, IMO, are obvious; if you are defining what it means for two objects to be equal, you should include all of the ways you can check for two objects to be equal. Note that the compiler doesn't (in general, cannot) enforce that you actually follow those rules. It's not going to perform complex code analysis of the body of your operators to see if they already mimic Equals because, in the worst case, that could be equivalent to solving the halting problem.

What it can do, however, is check for cases where you most likely are breaking those rules, in particular, you provided custom comparison operators and did not provide a custom Equals method. The assumption here is that you would not have bothered to provide operators if you did not want them to do something special, in which case, you should have provided custom behavior for all of the methods that need to be in sync.

If you did implement Equals to be something different from == the compiler would not complain; you would have hit the limit of how hard C# is willing to try to prevent you from doing something stupid. It was willing to stop you from accidentally introducing subtle bugs in your code, but it's going to let you purposely do so if that's what you want.

The third assumption has to do with the fact that many internal operations in the Framework use some variant of a hash table. If I have two objects that are, by my definition, "equal", then I should be able to do this:

if (a == b)
{
    var tbl = new HashTable();
    tbl.Add(a, "Test");

    var s = tbl[b];
    Debug.Assert(s.Equals("Test"));
}

This is a basic property of hash tables that would cause very strange problems if it were suddenly not true.

Michael Edenfield
  • 28,070
  • 4
  • 86
  • 117
3

My guess is your are getting these warning because compiler doesn't know that you use Equals in == method

Suppose you have this implementation

public struct  Person
{
    public int ID;
    public static bool operator ==(Person a, Person b) { return Math.Abs(a.ID - b.ID) <= 5; }
    public static bool operator !=(Person a, Person b) { return Math.Abs(a.ID - b.ID) > 5; }
}

Then

 Person p1 = new Person() { ID = 1 };
 Person p2 = new Person() { ID = 4 };

 bool b1 = p1 == p2;
 bool b2 = p1.Equals(p2);

b1 would be true, but b2 false

--EDIT--

Now suppose you want to do this

Dictionary<Person, Person> dict = new Dictionary<Person, Person>();
dict.Add(p1, p1);
var x1 = dict[p2]; //Since p2 is supposed to be equal to p1 (according to `==`), this should return p1

But this would throw an exception something like KeyNotFound

But if you add

public override bool Equals(object obj)
{
    return Math.Abs(ID - ((Person)obj).ID) <= 5; 
}
public override int GetHashCode()
{
    return 0;
}

you will get what you want.

The compiler just warns you that you can face with similar conditions

L.B
  • 114,136
  • 19
  • 178
  • 224
  • That's what I thought at first, but then, how does the situation change if I *do* override those methods? – user541686 May 28 '12 at 21:49
  • You could implement `Equals` method as `return Math.Abs(a.ID - b.ID) <= 5;` then all your code would be consistent. – L.B May 28 '12 at 21:53
  • Well I mean, if you're saying the problem is that *"compiler doesn't know that you use `Equals` in `==` method"*, then that problem still exists if I override `Equals` and/or `GetHashCode`... so what changed? – user541686 May 28 '12 at 21:55
1

All you need to do is add another member to your struct say Forename.

So if you has two persons with an ID of 63 but different forenames, are they equal or not?

All depends on what definition of "same" you want to implement.

Use a better example struct, write a noddy applictaion to execute the various methods and see what happens when you change the definitions of equality and or equivalence, if they aren't all in step, you end up with things like !(a == b) != (a != b), which might be true, but if you don't override all the methods whoever uses you code will wonder what your intent was.

Basically the compiler is telling you to be good citizen and make your intent clear.

Tony Hopkinson
  • 20,172
  • 3
  • 31
  • 39
  • +1 the last sentence (assuming it's correct) answers my question: you're saying it's just a clarity issue, not a correctness issue. – user541686 May 28 '12 at 21:50
  • No it's correctness as well. Your code is correct because you haven't changed what same means. If you had not overriding the other two methods would make them or it incorrect, and users of your code would have to guess. If I was peer reviewing your code, I'd be telling you take out your override, or questioning whether your override was correct – Tony Hopkinson May 28 '12 at 23:00
0

Probably because the default Equals() method is not expected to be good enough for a real system (e.g. in your class it should compare the ID field).

Rob I
  • 5,627
  • 2
  • 21
  • 28
  • 2
    But the compiler cannot be expected to know that is good enough. – Rob I May 28 '12 at 21:05
  • I don't understand what you mean by "good enough"... is there *ever* a `struct` where, if I don't override `Equals` or `GetHashCode`, overloading `==` and `!=` won't be "good enough"? (Could you give me an example please?) Thanks! – user541686 May 28 '12 at 21:10
0

If you override Equals and GetHashCode you wouldn't even need to override the operators, and that's a cleaner approach. Edited: it should work since this is a struct.

AD.Net
  • 13,352
  • 2
  • 28
  • 47
  • Would you mind expanding on your second point? How does it not work properly? – user541686 May 28 '12 at 21:07
  • .Equals() works well with value types, but not with reference types (classes) where it'll try check if two objects refer to the same instance, not the values inside (e.g. id) – AD.Net May 28 '12 at 21:10
  • Take a look at this link: http://stackoverflow.com/questions/1502451/c-what-needs-to-be-overriden-in-a-struct-to-ensure-equality-operates-properly – AD.Net May 28 '12 at 21:12
  • ... which explanation in that link are you referring to? – user541686 May 28 '12 at 21:14
  • The MSDN sample for equality check on structs – AD.Net May 28 '12 at 21:15
  • Right, I know that way works. My question is, what's wrong with *my* code? – user541686 May 28 '12 at 21:17
  • 1
    I don't think there's anything wrong and since it's a struct, I take back that it should work even without overriding equals. – AD.Net May 28 '12 at 21:24
0

Read the MSDN pages.

CS0660

CS0661

The compiler is basically saying: "Since you are saying know how to compare your object, you should make it compare that way all the time."

Kendall Frey
  • 43,130
  • 20
  • 110
  • 148
  • 1
    That's just begging my question though: Why does overloading `==` "imply" that I want to override the methods? – user541686 May 28 '12 at 21:09
  • 1
    @Mehrdad, it's not really that you want to. It's that the caller of your code wants consistency, and they also don't want unnecessary overrides. – Matthew Flaschen May 28 '12 at 21:11
  • 1
    1. Because calling code should be able to use == and Equals interchangeably. 2. If you want custom equality, why wouldn't you? – Kendall Frey May 28 '12 at 21:11
  • @KendallFrey: Why can't the calling code use `==` and `Equals` interchangeably in my example? – user541686 May 28 '12 at 21:20
  • @MatthewFlaschen: Okay if the actual answer is just 'consistency' then that works for me, thanks. :) I couldn't tell if it was a stylistic issue or a correctness issue. – user541686 May 28 '12 at 21:21
  • 1
    @Mehrdad: They can. Which implies that overriding == was a waste of time. – Kendall Frey May 28 '12 at 21:22
  • @KendallFrey: I guess it happened to be a waste of time here, but what if the field were a `string`? Then I *would* need to overload `==`, and yet everything would still work correctly, right? (I guess my example wasn't as ideal as it could've been, my bad... I fixed it.) – user541686 May 28 '12 at 21:23
  • @Mehrdad, my comment above is incorrect. It's *not* an unnecessary override, since the compiler doesn't provide an automatic `==` for user-defined value types. It is consistent, since the override defers to the existing `Equals`. So actually I think the original code is fine. – Matthew Flaschen May 28 '12 at 21:55
  • @MatthewFlaschen: Yeah I made the same exact mistake, thinking it was unnecessary in the case of an `int` field (and hence my edit)... funny. – user541686 May 28 '12 at 21:58
0
public struct Coord
{
    public int x;
    public int y;

    public Coord(int x, int y)
    {
        this.x = x;
        this.y = y;
    }

    public static bool operator ==(Coord c1, Coord c2)
    {
        return c1.x == c2.x && c1.y == c2.y;
    }

    public static bool operator !=(Coord c1, Coord c2)
    {
        return !(c1 == c2);
    }

    public bool Equals(Coord other)
    {
        return x == other.x && y == other.y;
    }

    public override bool Equals(object obj)
    {
        if (ReferenceEquals(null, obj)) return false;
        return obj is Coord && Equals((Coord) obj);
    }

    public override int GetHashCode()
    {
        return 0;
    }
}

Here's an example. Hopefully, it's helpful.

Arwego
  • 155
  • 3
  • 12