CODEWITHSUNDEEP
c#nulloperator-overloadingstack-overflow
Daniel Arkley
Daniel Arkley

Reputation: 174

Writing an operator overload in C#, for a complex type, which relies on the operator itself

I have a complex type called "HalfDay" which is a combination of a DateTime and an Enum value to represent either AM/PM.

public class HalfDay
{
  private DateTime _Date;
  public DateTime Date { get { return _Date; } set { _Date = value.Date; } }
  private DayHalf _Half;
  public DayHalf Half { get { return _Half; } set { _Half = value; } }
}

I am trying to write a set of overloaded operators to compare two HalfDays.

    public static bool operator <(HalfDay halfday1, HalfDay halfday2)
    {
        if (halfday1.Date.Date < halfday2.Date.Date) return true;
        if (halfday1.Date == halfday2.Date && halfday1.Half < halfday2.Half) return true;
        return false;
    }

    public static bool operator >(HalfDay halfday1, HalfDay halfday2)
    {
        if (halfday1.Date.Date > halfday2.Date.Date) return true;
        if (halfday1.Date == halfday2.Date && halfday1.Half > halfday2.Half) return true;
        return false;
    }

    public static bool operator ==(HalfDay halfday1, HalfDay halfday2)
    {
        if (halfday1 == null && halfday2 == null) return true;
        if (halfday1 == null || halfday2 == null) return false;

        return halfday1.Date.Date == halfday2.Date.Date && halfday1.Half == halfday2.Half;
    }

    public static bool operator !=(HalfDay halfday1, HalfDay halfday2)
    {
        if (halfday1 == null && halfday2 == null) return false;
        if (halfday1 == null || halfday2 == null) return true;

        return !(halfday1 == halfday2);
    }

In true newbie style (I've never written operators before) I've used the == comparison to compare a HalfDay with null. The result is, of course, a stack overflow as my operator code gets called recursively.

I think I need to check for null in my operators to correctly return false (or true) for comparisons being made where either of the arguments is null. I've clearly misunderstood something fundamental about OOP though - any pointers on how I should correctly write these operators?

Upvotes: 0

Views: 235

Answers (3)

svick
svick

Reputation: 244978

As others already mentioned, when you want to check for reference equality, bypassing any == or Equals(), use object.ReferenceEquals().

Also, when overloading ==, you should also override Equals() (and implement IEquatable<T>). When you do that, you can then take advantage of the fact that object.Equals(object, object) makes all the null checks and then invokes Equals(). So, your code could look like this:

public bool Equals(HalfDay other)
{
    if (object.ReferenceEquals(other, null))
        return false;
    if (object.ReferenceEquals(other, this))
        return true;

   return this.Date.Date == other.Date.Date && this.Half == other.Half;
}

public int GetHashCode()
{
    // TODO
}

public override bool Equals(object other)
{
    return Equals(other as HalfDay);
}

public static bool operator ==(HalfDay halfday1, HalfDay halfday2)
{
    return object.Equals(halfday1, halfday2);
}

public static bool operator !=(HalfDay halfday1, HalfDay halfday2)
{
    return !(halfday1 == halfday2);
}

Also, keep in mind what happens when there is a type that inherits from HalfDay. If you don't want to deal with that, make HalfDay sealed.

But, considering that you're also implementing comparison operators, you should considering unifying those too. For an example (with detailed explanation), see Eric Lippert's Math from scratch, part six: comparisons (you don't need to read the rest of the series).

Upvotes: 1

p.s.w.g
p.s.w.g

Reputation: 149050

Try using Object.ReferenceEquals instead:

public static bool operator ==(HalfDay halfday1, HalfDay halfday2)
{
    if (Object.ReferenceEquals(halfday1, halfday2)) return true;
    if (Object.ReferenceEquals(halfday1, null) || Object.ReferenceEquals(halfday2, null)) return false;

    return halfday1.Date.Date == halfday2.Date.Date && halfday1.Half == halfday2.Half;
}

Upvotes: 0

Habib
Habib

Reputation: 223362

See: Guidelines for Overloading Equals() and Operator == (C# Programming Guide)

A common error in overloads of operator == is to use (a == b), (a == null), or (b == null) to check for reference equality. This instead results in a call to the overloaded operator ==, causing an infinite loop. Use ReferenceEquals or cast the type to Object, to avoid the loop.

For comparing it against null you can do (Example code from documentation):

// If one is null, but not both, return false.
if (((object)a == null) || ((object)b == null))
{
    return false;
}

Also read:

By default, the operator == tests for reference equality by determining if two references indicate the same object, so reference types do not need to implement operator == in order to gain this functionality. When a type is immutable, meaning the data contained in the instance cannot be changed, overloading operator == to compare value equality instead of reference equality can be useful because, as immutable objects, they can be considered the same as long as they have the same value. Overriding operator == in non-immutable types is not recommended.

Upvotes: 2

Related Questions