Search code examples
c#equalityhashsetgethashcode

IEqualityComparer for Value Objects


I have an immutable Value Object, IPathwayModule, whose value is defined by:

  • (int) Block;
  • (Entity) Module, identified by (string) ModuleId;
  • (enum) Status; and
  • (entity) Class, identified by (string) ClassId - which may be null.

Here's my current IEqualityComparer implementation which seems to work in a few unit tests. However, I don't think I understand what I'm doing well enough to know whether I am doing it right. A previous implementation would sometimes fail on repeated test runs.

private class StandardPathwayModuleComparer : IEqualityComparer<IPathwayModule>
{
    public bool Equals(IPathwayModule x, IPathwayModule y)
    {
        int hx = GetHashCode(x);
        int hy = GetHashCode(y);
        return hx == hy;
    }

    public int GetHashCode(IPathwayModule obj)
    {
        int h;
        if (obj.Class != null)
        {
            h = obj.Block.GetHashCode() + obj.Module.ModuleId.GetHashCode() + obj.Status.GetHashCode() + obj.Class.ClassId.GetHashCode();
        }
        else
        {
            h = obj.Block.GetHashCode() + obj.Module.ModuleId.GetHashCode() + obj.Status.GetHashCode() + "NOCLASS".GetHashCode();
        }
        return h;
    }
}

IPathwayModule is definitely immutable and different instances with the same values should be equal and produce the same HashCode since they are used as items within HashSets.

I suppose my questions are:

  • Am I using the interface correctly in this case?
  • Are there cases where I might not see the desired behaviour?
  • Is there any way to improve the robustness, performance?
  • Are there any good practices that I am not following?

Solution

  • Thanks to all who responded. I have aggregated the feedback from everyone who responded and my improved IEqualityComparer now looks like:

    private class StandardPathwayModuleComparer : IEqualityComparer<IPathwayModule>
    {
        public bool Equals(IPathwayModule x, IPathwayModule y)
        {
            if (x == y) return true;
            if (x == null || y == null) return false;
    
            if ((x.Class == null) ^ (y.Class == null)) return false;
    
            if (x.Class == null) //and implicitly y.Class == null
            {
                return x.Block.Equals(y.Block) && x.Status.Equals(y.Status) && x.Module.ModuleId.Equals(y.Module.ModuleId);
            }
            return x.Block.Equals(y.Block) && x.Status.Equals(y.Status) && x.Module.ModuleId.Equals(y.Module.ModuleId) && x.Class.ClassId.Equals(y.Class.ClassId);
        }
        public int GetHashCode(IPathwayModule obj)
        {
            unchecked {
                int h = obj.Block ^ obj.Module.ModuleId.GetHashCode() ^ (int) obj.Status;
                if (obj.Class != null)
                {
                   h ^= obj.Class.ClassId.GetHashCode();
                }
                return h;
            }
        }
    }