Have I changed the intent of the overridden `==` operator?

Question

The following overloaded ==operator is part of the Calender class in QL.net

    public static bool operator ==(Calendar c1, Calendar c2)
    {
        return (c1.empty() && c2.empty()) 
                || (!c1.empty() && !c2.empty() && c1.name() == c2.name());
    }

    public bool empty() { return (object)calendar == null; }

When I try to access the SouthAfricanCalender property, I receive a System.NullReferenceException : Object reference not set to an instance of an object. which prompted me to dig into the source.

public SouthAfrica SouthAfricanCalender
{
    get
    {
        if (_calender == null)
        {
        _calender = new SouthAfrica();
        }
    return _calender;
    }
    set
    {
        if (_calender == null)
        { 
        _calender = value;
        }
    }
}

SouthAfrica _calender;

I have ammended the overload as follows based on the answer here

public static bool operator ==(Calendar c1, Calendar c2)
{
   if ( object.ReferenceEquals(c1,c2)) return true;
   if ((object)c1 == null || (object)c2 == null) return false;

   return (c1.empty() && c2.empty())
       || (!c1.empty() && !c2.empty() && c1.name() == c2.name());
}

My question, have I changed the intent of the original code with my amendment?

Edit: any suggestions on how this can be cleaned up further?

You dont need `if ((object)c1 == null || (object)c2 == null) return false;` no casting to object required. only `if c1 == null || (c2 == null) return false;` — Aliostad, Dec 15 '10 at 13:25
@aliostad: Not true! Without the cast to `object` you'll potentially get an infinite loop of `==` calls leading to a `StackOverflowException`. — LukeH, Dec 15 '10 at 13:34
In your updated == method, after the first two if statements, you've already determined that both c1 and c2 are non-null. You can change your return statement to `return c1.name() == c2.name();` — David Yaw, Dec 15 '10 at 16:44
@david - i was thinking the same - just need to make sure that nothing weird happening in the `name()` function. — Ahmad, Dec 15 '10 at 17:26

score 1 · Answer 1 · answered Dec 15 '10 at 13:26

1

No, you haven't.

It still checks for equality.

answered Dec 15 '10 at 13:26

SLaks

868,454
176
1,908
1,964

score 1 · Accepted Answer · answered Dec 15 '10 at 13:26

No. You ensure that both are objects, and respond accordingly in the places that they aren't (assuming that ReferenceEquals can handle double null). Then you simply execute the same check. The whole .empty() thing is totally unnecessary, by the way, you already know that it's not null, just return the name comparison.

Have I changed the intent of the overridden `==` operator?

2 Answers2