Comment 4 for bug 1157828

Revision history for this message
Meg Gotshall (natraj) wrote :

I think the biggest reason to fix it would be for consistency. Right now, if you override the equality operation, it will use the custom comparison EXCEPT for the case where one side is null. That means that you would essentially need to document the Assert.AreEqual method to say that it uses the custom comparison for types that override Object::Equals but performs a reference comparison when 'actual' or 'expected' is null. As an API developer, this sounds like a documentation nightmare. And since most programmers aren't inclined to read the documentation anyway, once they see that it uses their custom implementation once, they will assume that it always uses the custom implementation.

I'm not thrilled with the NullObject API I'm using, but its documentation recommends using 'resultObj != null' as the null check even in cases where the NullObject might be returned. Having written 'if (resultObje != null)' all over production code, I instinctively wrote 'Assert.IsNotNull(resultObj)' in my test code. Only to have them fail unexpectedly later on when trying to perform an unsupported operation on the NullObject. I can completely back the idea that IsNull and IsNotNull should be reference comparisons to the null pointer, but with the same Equality logic underneath, it is closely coupled to the AreEqual ambiguity.

Chances are that a caller isn't going to test every particular circumstance. If I have a type that overrides .Equals, I would run a quick test to see if something like AreEqual is using reference comparison or calling my custom comparison. After that first check I would assume (never a good idea, but not uncommon) that the method will always use the custom comparison.

And evidence indicates that at least one otherwise talented developer has fallen into this trap already! ;)