patterncsharpModerate
Comparing Equals() method from MSDN
Viewed 0 times
comparingmethodequalsmsdnfrom
Problem
I've implemented the
This obviously doesn't match the reference implementation on MSDN, here, as they add explicit checking for null before and after converting to a
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?
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
Some comments however:
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
IEquatableMSDN
- There should be no performance difference unless you have an unusual amount of
nullarguments toEquals(object)that will now go through theasstatement 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.