What should GetHashCode return when object's identifier is null?

Dienekes picture Dienekes · Feb 22, 2011 · Viewed 18.3k times · Source

Which of the following is correct/better, considering that identity property could be null.

public override int GetHashCode()
{
    if (ID == null) {
        return base.GetHashCode();
    }
    return ID.GetHashCode();
}

OR

public override int GetHashCode()
{
    if (ID != null) {
        return ID.GetHashCode();
    }
     return 0;
}

Update 1: Updated 2nd option.

Update 2: Below are the Equals implementations:

public bool Equals(IContract other)
{
    if (other == null)
        return false;
    if (this.ID.Equals(other.ID)) {
        return true;
    }
    return false;
}

public override bool Equals(object obj)
{
    if (obj == null)
        return base.Equals(obj);
    if (!obj is IContract) {
        throw new InvalidCastException("The 'obj' argument is not an IContract object.");
    } else {
        return Equals((IContract)obj);
    }
}

And ID is of string type.

Answer

Jon Skeet picture Jon Skeet · Feb 22, 2011

It really depends on what you want equality to mean - the important thing is that two equal objects return the same hashcode. What does equality mean when ID is null? Currently your Equals method would have to return true if the ID properties have the same value... but we don't know what it does if ID is null.

If you actually want the behaviour of the first version, I'd personally use:

return ID == null ? base.GetHashCode() : ID.GetHashCode();

EDIT: Based on your Equals method, it looks like you could make your GetHashCode method:

return ID == null ? 0 : ID.GetHashCode();

Note that your Equals(IContract other) method could also look like this:

return other != null && object.Equals(this.ID, other.ID);

Your current implementation will actually throw an exception if this.ID is null...

Additionally, your Equals(object) method is incorrect - you shouldn't throw an exception if you're passed an inappropriate object type, you should just return false... ditto if obj is null. So you can actually just use:

public override bool Equals(object obj)
{
    return Equals(obj as IContract);
}

I'm concerned about equality based on an interface, however. Normally two classes of different types shouldn't be considered to be equal even if the implement the same interfaces.