I am getting stack overflow in this apparently benign code:

    public static bool operator == (AccessLevel al1, AccessLevel al2)
    {
        if (al1 == null && al2 == null)
            return true;

        if (al1 == null || al2 == null)
            return false;

        if (al1._Value == al2._Value)
            return true;
        else
            return false;
    }
    public static bool operator != (AccessLevel al1, AccessLevel al2)
    {
        if (al1 == null && al2 == null)
            return false;

        if (al1 == null || al2 == null)
            return true;

        if (al1._Value != al2._Value)
            return true;
        else
            return false;
    }

What appears to be happening is that in the test for null in operator overload, the == overload is called again, and again, and again until the stack wind up. Possible solutions here is to use the "Equals" override, which would look like Equals(null) returning true... but how to code that without using the == overload ? Is this the correct implementation:

        public override bool Equals(object o)
        {
            if (o == null)
                return true;
            if ((AccessLevel)o == this)
                return true;
            return false;
        }

Or could one say something like this:

        public static bool operator == (AccessLevel al1, AccessLevel al2)
        {
            if (al1 is null && al2 is null)
                return true;

            if (al1 is null || al2 is null)
                return false;

            if (al1._Value == al2._Value)
                return true;
            else
                return false;
        }

Any insight on how to correctly handle this?

CodePudding user response：

To implement equality tests "properly", use:

public class AccessLevel : IEquatable<AccessLevel>
{
    ...
    
    public override int GetHashCode() => _Value.GetHashCode();
    
    public override bool Equals(object obj) 
        => obj is AccessLevel other 
        && Equals(other);
    
    public bool Equals(AccessLevel other)
    {
        if (other is null) return false;
        if (ReferenceEquals(this, other)) return true;
        return _Value == other._Value;
    }
    
    public static bool operator ==(AccessLevel left, AccessLevel right)
        => Equals(left, right);
    
    public static bool operator !=(AccessLevel left, AccessLevel right)
        => !Equals(left, right);
}

CodePudding user response：

Your first option would be semantically incorrect, as well as inefficient. Equals(null) should never return true. As per the documentation:

The following statements must be true for all implementations of the Equals(Object) method. [...]

...

• x.Equals(null) returns false.

Your second approach of using is is correct and more idiomatic in modern C#, although I'd argue the final part would be clearer as just:

return al1._Value == al2._Value;

Your Equals method can then just be implemented as:

public override bool Equals(object o) => this == o as AccessLevel;

(It's up to you whether the semantics are implemented in the == operator or in the Equals method, but it's best to implement them in just one of the two places to avoid duplication and potential inconsistency.)