Custom GUID always return false on object.Equals

Question

We have GUIDs as identifiers in our systems. As it's easy to mess up and pass the id of one entity into a method that expects the id of another entity (lets say you pass the OrderId to the InvoiceId by mistake because it's all Guids) we created our own types for Guids, so the compiler can easily tell me "hey, don't pass an OrderId here, I expect an InvoiceId".

So basically, we have lots of wrappers around Guid. Those wrappers work well, they are basically copies of the Guid interface delegating all the work to their internally stored Guid.

One thing that I cannot figure out is that Assert.AreEqual(a, b) on two of our custom identifiers will fail. It calls object.Equals(a, b) that in turn calls a == b and that will not call my operator == but instead call something else and return false. It does not for Guid though and I cannot figure out what I missed.

What do I need to implement for my custom types to actually work and return true on object.Equals(a, b) given that it already does on operator ==?

namespace ConsoleApp13
{
    using System;
    using System.Runtime.InteropServices;

    //// Same signature, interfaces and and attributes as
    //// https://referencesource.microsoft.com/#mscorlib/system/guid.cs

    [StructLayout(LayoutKind.Sequential)]
    [Serializable]
    [ComVisible(true)]
    // not accessible for me: [System.Runtime.Versioning.NonVersionable]
    public struct CustomId : IFormattable, IComparable, IComparable<CustomId>, IEquatable<CustomId>
    {
        public static readonly CustomId Empty = new CustomId();

        private readonly Guid internalGuid;

        private CustomId(Guid guid)
        {
            this.internalGuid = guid;
        }

        public static bool operator ==(CustomId a, CustomId b)
        {
            return a.internalGuid == b.internalGuid;
        }

        public static bool operator !=(CustomId a, CustomId b)
        {
            return !(a.internalGuid == b.internalGuid);
        }

        public static CustomId NewGuid()
        {
            return new CustomId(Guid.NewGuid());
        }

        public static implicit operator Guid(CustomId value)
        {
            return value.internalGuid;
        }

        public static explicit operator CustomId(Guid value)
        {
            return new CustomId(value);
        }

        public override string ToString()
        {
            return "[" + this.GetType().Name + ":" + this.internalGuid.ToString("D") + "]";
        }

        public override int GetHashCode()
        {
            return this.internalGuid.GetHashCode();
        }

        public override bool Equals(object obj)
        {
            return this.internalGuid.Equals(obj);
        }

        public bool Equals(CustomId other)
        {
            return this.internalGuid.Equals(other.internalGuid);
        }

        public int CompareTo(object obj)
        {
            return this.internalGuid.CompareTo(obj);
        }

        public int CompareTo(CustomId other)
        {
            return this.internalGuid.CompareTo(other.internalGuid);
        }

        public string ToString(string format, IFormatProvider formatProvider)
        {
            return this.internalGuid.ToString(format, formatProvider);
        }
    }

    internal static class Program
    {
        internal static void Main()
        {
            {
                var a = CustomId.NewGuid();
                var b = a;

                // shows true false
                Console.WriteLine("{0} {1}", a == b, object.Equals(a, b));
            }

            {
                var a = Guid.NewGuid();
                var b = a;

                // shows true true
                Console.WriteLine("{0} {1}", a == b, object.Equals(a, b));
            }

            Console.WriteLine(@"Done.");
            Console.ReadLine();
        }
    }
}

yeah, I'd ignore that comment - this is a perfect scenario for a struct; although I *would* be very tempted to declare it as a `readonly struct` if you're using recent C# version. Almost all structs should be `readonly struct`. — Marc Gravell, Jul 27 '18 at 13:49

score 4 · Accepted Answer · answered Jul 27 '18 at 13:44

Your code here:

    public override bool Equals(object obj)
    {
        return this.internalGuid.Equals(obj);
    }

is going to unwrap itself, but it doesn't unwrap the other instance, so : it will always fail if obj is a CustomId, as the Guid won't expect to be handed a CustomId (it wants a Guid). Perhaps this should be:

    public bool Equals(object obj) => obj is CustomId cid && cid == this;

Note that CompareTo should probably be similar:

    public int CompareTo(object obj) => obj is CustomId cid ? this.CompareTo(cid) : -1;

score 1 · Answer 2 · answered Jul 27 '18 at 13:50

The answer from Marc Gravell is correct but I want to note something.

It calls object.Equals(a, b) that in turn calls a == b

This is a wrong assumption. object.Equals(a, b) will call a.Equals(b) if both of them are not null. Subtle difference but it is a difference:

https://referencesource.microsoft.com/#mscorlib/system/object.cs,d9262ceecc1719ab

public static bool Equals(Object objA, Object objB) 
{
    if (objA==objB) {
        return true;
    }
    if (objA==null || objB==null) {
        return false;
    }
    return objA.Equals(objB);
}

True, I did not look beyond that `==` and never looked to closely at `.Equals` — nvoigt, Jul 27 '18 at 13:51

Custom GUID always return false on object.Equals

2 Answers2