1

There is a nice explanation why object.ReferenceEquals(this, obj) must not be used for value types:

When comparing values using ReferenceEquals, if objA and objB are value types, they are boxed before they are passed to the ReferenceEquals method. This means that even if both objA and objB represent the same instance of a value type, the ReferenceEquals method nevertheless returns false

However, I found code similar to the following snippet:

internal readonly struct Foo
{
    public override bool Equals(object obj)
    {
        if (object.ReferenceEquals(this, obj))
        {
            return true;
        }

        if (obj is Foo other)
        {
            return this.Equals(other);
        }

        return false;
    }

    public bool Equals(Foo other)
    {
        // ..
        return true;
    }
}

Is this just wrong or is there an edge case in which ReferenceEquals could be useful (i.e., evaluate to true) for value types, especially structs?

momo
  • 3,313
  • 2
  • 19
  • 37
  • 1
    Yeah, it's just wrong. `Foo` is a value type, so passing `this` to `object.ReferenceEquals` will box the instance itself. The newly boxed instance can't be equal to anything because that reference never existed before `Equals(object)` was called. – madreflection Apr 08 '21 at 15:20
  • Yes, it is wrong. The StackOverflowException it generates ensures it won't be used. – Hans Passant Apr 08 '21 at 15:23
  • Assuming there's no `Equals(Foo)` that just isn't included here. – madreflection Apr 08 '21 at 15:24
  • I guess it's possible it might not throw a `StackOverflowException` if there's an overload `bool Equals(Foo obj)`. I guess. –  Apr 08 '21 at 15:25
  • Yes, the `Equals(Foo obj)` implementation is omitted here. – momo Apr 08 '21 at 15:42
  • @momo Can you add it to the question? It's relevant, considering the comments. –  Apr 08 '21 at 15:44
  • for reference `obj is Foo other` is the [`is` type pattern matching](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/is#type-pattern) [introduced in C# 7](https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-7#pattern-matching) – bolov Apr 08 '21 at 16:19

1 Answers1

4

It shouldn't be. The code as-written is simply bad, in that it unnecessarily boxes something for a test that will never work. For completeness, a more useful and idiomatic pattern in this case would be:

// custom Foo equality implementation
public bool Equals(Foo other)
{
    // ...
}
// default object implemenentation
public override int GetHashCode() {...} // must match Equals(Foo) logic
public override bool Equals(object obj) => obj is Foo other && Equals(other);
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900