0

Consider, as an example, the following minimalistic Point class, which works with doubles:

public class Point
{
  public double X { get; set; }
  public double Y { get; set; }

  public Point(double x, double y)
  {
    X = x;
    Y = y;
  }

  public static Point operator +(Point left, Point right)
  {
    return new Point(left.X + right.X, left.Y + right.Y);
  }

  public static Point operator -(Point left, Point right)
  {
    return new Point(left.X - right.X, left.Y - right.Y);
  }
}

The plus and minus operators make perfect sense to me. Running the Visual Studio 2013 Code Analysis, I get the warning CA1013: Overload operator equals on overloading add and subtract. I would implement this whole equality-thing as follows:

public static bool operator ==(Point point1, Point point2)
{
  if (object.ReferenceEquals(point1, point2))
    return true;

  if (object.ReferenceEquals(null, point1))
    return false;

  return point1.Equals(point2);
}

public static bool operator !=(Point point1, Point point2)
{
  return !(point1 == point2);
}

public override bool Equals(object obj)
{
  if (object.ReferenceEquals(null, obj))
    return false;

  if (object.ReferenceEquals(this, obj))
    return true;

  if (obj.GetType() != this.GetType())
    return false;

  return this.IsEqual((Point)obj);
}

public bool Equals(Point point)
{
  if (object.ReferenceEquals(null, point))
    return false;

  if (object.ReferenceEquals(this, point))
    return true;

  return this.IsEqual(point);
}

private bool IsEqual(Point point)
{
  return this.X == point.X && this.Y == point.Y;
}

public override int GetHashCode()
{
  return 571 + 821 * this.X.GetHashCode() + 307 * this.Y.GetHashCode();
}

However, when comparing two points, I would not use that kind of comparison, but rather compare the Points within a tolerance, since I am working with doubles. For this, I could use a 'NearlyEqual'-Method or something like that, but not '==' or 'Equals', since those methods have to be transitive.

Should I suppress the warning in a case like this, or are there any reasons why it could still make sense to implement 'Equals'?

Timitry
  • 2,646
  • 20
  • 26
  • You may also want to implement IEquatalbe, IMO. If you will use them in a collection or you will need to order them then you'd probably will enjoy Equal & friends. Also GetHashCode() and you may finally consider if it has to be a struct or a class... – Adriano Repetti May 18 '16 at 13:37
  • @AdrianoRepetti: Interesting point (pun intended) with IEquatable and sorting. Do you maybe want to elaborate on that in an answer? – Timitry May 18 '16 at 13:56
  • Little bit busy now but I'll be happy to write an answer later if no one else will expand these _points_. – Adriano Repetti May 18 '16 at 14:03
  • @AdrianoRepetti (or others): I added an implementation of Equals, == and != to the original post (hope it's correct). I'd still be interested in an answer to the question of usefulness, if maybe now you've got some time! – Timitry Sep 15 '16 at 07:43
  • My opinion: You should follow the norm of the other floating point types in .NET and implement bit-for-bit equality checks (if at all) and leave the "near enough" checks to the user of your code, possibly by providing a helper function that lets you specify the tolerance to use. I would not implement the tolerant check inside the object in the equality check methods and operators. – Lasse V. Karlsen Sep 15 '16 at 07:47
  • @LasseV.Karlsen: As stated in the question, I would not use Equals for a tolerance check, and provide a helper method (NearlyEqual) anyway. The real question is in which scenarios a bit-for-bit equality check could be useful - and whether to implement it or not, as Code Analysis suggests. – Timitry Sep 15 '16 at 07:53
  • Well, code analysis rules are guidelines, not laws, they are meant to get you to stop and think to ensure you implement things you would normally perhaps forget or do incorrectly. If you don't need the equality operators I'd say you've thought through it, so ask the rule engine to ignore this. To be honest, I wouldn't implement equality checks *just to make code analysis happy*. Either you need them or you don't, it made you stop and think, so what do you think? Do you need them? – Lasse V. Karlsen Sep 15 '16 at 07:55
  • I agree with Lasse, if there is such operator you _may_ use it in error. If there is not then you will think twice which equal function you need (we have to live with bugged code because of == with strings). I'd however override Equals() because default implementation is not appropriate, equal comparison for floating points is not optimal but reference comparison of instances is even more astonishing. Personal note: for a non-sealed reference type I never ever provide == and != implementations – Adriano Repetti Sep 15 '16 at 08:02
  • @LasseV.Karlsen: You're right, I also wouldn't implement something just because code analysis says so. In my opinion, I do not need the Equality-Checks, since I usually compare with a tolerance. However, when I posted the question, I only started coding on a professional basis three months ago, so I was wondering if I might be missing something - e.g. some nice functionality, like AdrianoRepetti pointed out. – Timitry Sep 15 '16 at 08:03
  • Also remember that == and Equals() are different and independent things.I'd provide an implementation for Equals() (and then also for GetHashCode()) because default implementation is obviously wrong (in this case) for a reference type and terribly slow for a value type. Better an implementation that _should not be used_ than a wrong one. Also note that Equals (and also IEquatable implementation) may provide a useful default tolerance value (you just need to document it). – Adriano Repetti Sep 15 '16 at 08:11
  • In this case check how System.Drawing.PointF has been implemented: it (wrongly) use == for double comparison but, after all, if caller uses Equals() is not aware of floating point errors then it may also do point1.X == point2.X... – Adriano Repetti Sep 15 '16 at 08:14

0 Answers0