I have a scenario when I want to have a HashSet<ITrackableObjectData>
, where ITrackableObjectData
has string TypeName
and object Id
readonly immutable properties. Id
is a primitive value, ie. string
, int
, long
, Guid
etc.
I want to use these equality rules:
- If the
TypeName
property is different, should anyway be considered non-equal. - If the
Id
is zero, null etc, should use the base object hashcode (reference equal). - If
Id
is not zero, then it should compare theTypeName
andId
.
I created the following comparer to provided to the HashSet
's constructor, and I'd like to hear from you of any improvements/vandalism in my code. As you can see I'm using the GetHashCode
in the Equals
method, is this a bad-idea?
Note that these are my equality requirements in my scenario.
class ITrackableObjectDataComparer : EqualityComparer<ITrackableObjectData>
{
public override bool Equals(ITrackableObjectData x, ITrackableObjectData y) =>
GetHashCode(x) == GetHashCode(y);
public override int GetHashCode(ITrackableObjectData obj)
{
if (obj == null) return 0;
var typeHash = obj.TypeName == null ? 0 : obj.TypeName.GetHashCode();
var idHash = obj.Id == null ? 0 : obj.Id.GetHashCode();
idHash = idHash == 0 ? obj.GetHashCode() : idHash;
return typeHash ^ idHash;
}
}
Note that this EqualityComparer
is a private class and is only meant to serve the purpose of this specific HashSet
which is not exposed at all.
In fact I don't also really care about the Equals
, but rather about the GetHashCode
, so I know whether an object with my specs have already been added to a HashSet
.
Update:
This is basically the equality checker that fulfills my requirements (but again, I don't need an equality comparer, I need a hash equality-comparer)
public override bool Equals(ITrackableObjectData x, ITrackableObjectData y)
{
if (x == y) return true; //ref eq.
else if (x == null)
return y == null;
else if (y == null)
return false;
else if (x.TypeName != y.TypeName)
return false;
else
{
if (x.Id == null)
return y.Id == null;
else if (y.Id == null)
return false;
else
{
int xId = x.Id.GetHashCode(),
yId = y.Id.GetHashCode();
if (xId == 0 || yId == 0)
return x.Equals(y); //use default eq.
else
return xId == yId;
}
}
}
ITrackableObjectData
and one of its implementations... Especially given the nullability of both values involved. \$\endgroup\$ – Mathieu Guindon♦ Jun 5 '17 at 13:46Equals
asif (x.GetHashcode() != y.GetHashcode()) return false; else ...
but there has to be something in thatelse
that deals with the situation where there are equal hash codes but unequal objects. \$\endgroup\$ – Eric Lippert Jun 5 '17 at 21:13Equals
implementation does not follow the requirements you listed. It's just wrong and it won't work. It might work most of the time due to pure luck, but sooner or later you will lose objects due to hashcode collisions, which your implementation does not account for. You might not care about it, but users of your software most likely will when they lose their data. Whether or not this class isprivate
is completely irrelevant. Private class should work correctly too. \$\endgroup\$ – Nikita B Jun 5 '17 at 23:12