Equals/GetHashCode override warning in derived class with no state

Question

I've created a strongly-typed, immutable wrapper class for various string IDs that flow through our system

The abstract BaseId class:

(some error-checking and formatting omitted for brevity...)

public abstract class BaseId
{
    // Gets the type name of the derived (concrete) class
    protected abstract string TypeName { get; }

    protected internal string Id { get; private set; }

    protected BaseId(string id) { Id = id; }

    // Called by T.Equals(T) where T is a derived type
    protected bool Equals(BaseId other)
    {
        if (ReferenceEquals(null, other))
            return false;
        if (ReferenceEquals(this, other))
            return true;
        return String.Equals(Id, other.Id);
    }

    // warning CS0660 (see comment #1 below)
    //public override bool Equals(object obj) { return base.Equals(obj); }

    public override int GetHashCode()
    {
        return TypeName.GetHashCode() * 17 + Id.GetHashCode();
    }

    public override string ToString()
    {
        return TypeName + ":" + Id;
    }

    // All T1 == T2 comparisons come here (where T1 and T2 are one
    // or more derived types)
    public static bool operator ==(BaseId left, BaseId right)
    {
        // Eventually calls left.Equals(object right), which is
        // overridden in the derived class
        return Equals(left, right);
    }

    public static bool operator !=(BaseId left, BaseId right)
    {
        // Eventually calls left.Equals(object right), which is
        // overridden in the derived class
        return !Equals(left, right);
    }
}

My goal was keep as much of the implementation in the base class so that the derived classes would be small, consisting mostly/entirely of boilerplate code.

Example concrete DerivedId class:

Note that this derived type defines no additional state of its own. Its purpose is solely to create a strong type.

public sealed class DerivedId : BaseId, IEquatable<DerivedId>
{
    protected override string TypeName { get { return "DerivedId"; } }

    public DerivedId(string id) : base(id) {}

    public bool Equals(DerivedId other)
    {
        // Method signature ensures same (or derived) types, so
        // defer to BaseId.Equals(object) override
        return base.Equals(other);
    }

    // Override this so that unrelated derived types (e.g. BarId)
    // NEVER match, regardless of underlying Id string value
    public override bool Equals(object obj)
    {
        // Pass obj or null for non-DerivedId types to our
        // Equals(DerivedId) override
        return Equals(obj as DerivedId);
    }

    // warning CS0659 (see comment #2 below)
    //public override int GetHashCode() { return base.GetHashCode(); }
}

Each class is generating a compiler warning:

1. Not overriding Object.Equals(object o) in BaseId generates a compile warning:
   
   warning CS0660: 'BaseId' defines operator == or operator != but does not override Object.Equals(object o)
   
   But if I implement BaseId.Equals(object o), it'll just be to call the base class implementation in Object.Equals(object o). I don't see how this will ever get called anyway; it's always overridden in the derived class, and the implementation there doesn't call up to this implementation.

2. Not overriding BaseId.GetHashCode() in DerivedId generates a compile warning:
   
   warning CS0659: 'DerivedId' overrides Object.Equals(object o) but does not override Object.GetHashCode()
   
   This derived class has no additional state, so there's nothing for me to do in an implementation of DerivedId.GetHashCode(), except to call the base class implementation in BaseId.GetHashCode().
   
   

I can suppress the compiler warnings or just implement the methods and have them call the base class implementations, but I want to make sure I'm not missing something.

Is there something odd about the way I did this, or is this just one of those things that you have to do to suppress warnings on otherwise correct code?

Answer 1

The reason those are warnings rather than errors is that the code will still work (probably), but it might do things that you don't expect. The warning is a big red flag that says, "Hey! You might be doing something bad here. You might want to take another look at it."

As it turns out, the warning is right on.

In this particular case, it's possible that some code can call Object.Equals(object) on one of your BaseId objects. For example, somebody could write:

bool CompareThings(BaseId thing, object other)
{
    return thing.Equals(other);
}

The compiler will generate a call to Object.Equals(object) because your BaseId type doesn't override it. That method will do the default comparison, which is the same as Object.ReferenceEquals(object). So you have two different meanings of Equals. You need to override Object.Equals(object) and have it call Equals(BaseId) after checking that the object being compared is indeed of type BaseId.

In the second case, you're right: there probably isn't a need to override GetHashCode, since the object doesn't define any new fields or do anything that changes the meaning of Equals. But the compiler doesn't know that. Sure, it knows that you didn't add any fields, but you did override Equals, meaning that you potentially changed the meaning of equality. And if you changed the meaning of equality, then you very likely changed (or should change) how hash codes are computed.

Not handling equality properly is a very common cause of mistakes when designing new types. It's a good thing that the compiler is overly cautious in this area.