HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Comparing Equals() method from MSDN

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
comparingmethodequalsmsdnfrom

Problem

I've implemented the Equals() support for my class as follows:

public override bool Equals(object obj)
{
  return Equals(obj as TwoDPoint);
}

public bool Equals(TwoDPoint p)
{
  return ((object)p != null) && (x == p.x) && (y == p.y);
}


This obviously doesn't match the reference implementation on MSDN, here, as they add explicit checking for null before and after converting to a TwoDPoint, and they implement the actual equality test in each method (i.e. no call from one to the other).

I like my implementation as it's much more succinct. But I'm wondering - what have I missed? Am I losing (significant) performance with my version? Is there actually a bug in this approach?

Solution

I like my implementation as it's much more succinct

Which isn't necessarily a good thing: we care about readable code, not about oneliners. It means we don't use 5 lines to write what can be written in 1 line but it also means we don't cram 5 lines in 1 line just for the sake of it.

That being said: your implementation does the same as the example. I would also prefer your example since it keeps the equality logic in one method Equals(T) rather than duplicating it.

Some comments however:

  • There's no point in doing this cast: (object)p != null



  • You might as well implement IEquatable MSDN



  • There should be no performance difference unless you have an unusual amount of null arguments to Equals(object) that will now go through the as statement before being caught by the null check



  • Equals(T) will probably be inlined anyway

Context

StackExchange Code Review Q#122488, answer score: 13

Revisions (0)

No revisions yet.