Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why ICollection<>.Contains ignores my overridden Equals and the IEquatable<> interface?

I have an issue with a navigation property in an entity framework project.

Here is the class MobileUser:

[DataContract]
[Table("MobileUser")]
public class MobileUser: IEquatable<MobileUser>
{
    // constructors omitted....

    /// <summary>
    /// The primary-key of MobileUser.
    /// This is not the VwdId which is stored in a separate column
    /// </summary>
    [DataMember, Key, Required, DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int UserId { get; set; }

    [DataMember, Required, Index(IsUnique = true), MinLength(VwdIdMinLength), MaxLength(VwdIdMaxLength)]
    public string VwdId { get; set; }

    // other properties omitted ...

    [DataMember]
    public virtual ICollection<MobileDeviceInfo> DeviceInfos { get; private set; }

    public bool Equals(MobileUser other)
    {
        return this.UserId == other?.UserId || this.VwdId == other?.VwdId;
    }

    public override bool Equals(object obj)
    {
        if(object.ReferenceEquals(this, obj))return true;
        MobileUser other = obj as MobileUser;
        if (other == null) return false;
        return this.Equals(other);
    }

    public override int GetHashCode()
    {
        // ReSharper disable once NonReadonlyMemberInGetHashCode
        return VwdId.GetHashCode();
    }

    public override string ToString()
    {
        return "foo"; // omitted actual implementation
    }

    #region constants
    // irrelevant
    #endregion
}

The relevant part is this navigation property:

public virtual ICollection<MobileDeviceInfo> DeviceInfos { get; private set; }

This is the class MobileDeviceInfo:

[DataContract]
[Table("MobileDeviceInfo")]
public class MobileDeviceInfo : IEquatable<MobileDeviceInfo>
{
    [DataContract]
    public enum MobilePlatform
    {
        [EnumMember]
        // ReSharper disable once InconsistentNaming because correct spelling is iOS
        iOS = 1,
        [EnumMember] Android = 2,
        [EnumMember] WindowsPhone = 3,
        [EnumMember] Blackberry = 4
    }

    // constructors omitted ...

    [DataMember, Key, DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int DeviceInfoId { get; private set; }

    [DataMember, Required, Index(IsUnique = true), MinLength(DeviceTokenMinLength), MaxLength(DeviceTokenMaxLength)]
    public string DeviceToken { get; set; }

    [DataMember, Required, MinLength(DeviceNameMinLength), MaxLength(DeviceNameMaxLength)]
    public string DeviceName { get; set; }

    [DataMember, Required]
    public MobilePlatform Platform { get; set; }

    // other properties ...

    [DataMember]
    public virtual MobileUser MobileUser { get; private set; }

    /// <summary>
    ///     The foreign-key to the MobileUser.
    ///     This is not the VwdId which is stored in MobileUser
    /// </summary>
    [DataMember, ForeignKey("MobileUser")]
    public int UserId { get; set; }

    public bool Equals(MobileDeviceInfo other)
    {
        if (other == null) return false;
        return DeviceToken == other.DeviceToken;
    }

    public override string ToString()
    {
        return "Bah"; // implementation omitted

    public override bool Equals(object obj)
    {
        if (ReferenceEquals(this, obj)) return true;
        MobileDeviceInfo other = obj as MobileDeviceInfo;
        if (other == null) return false;
        return Equals(other);
    }

    public override int GetHashCode()
    {
        // ReSharper disable once NonReadonlyMemberInGetHashCode
        return DeviceToken.GetHashCode();
    }

    #region constants
    // irrelevant
    #endregion
}

As you can see, it implements IEquatable<MobileDeviceInfo> and overrides also Equals and GetHashCode from System.Object.

I have following test, i've expected that Contains would call my Equals but it does not. It seems to use Object.ReferenceEquals instead, so won't find my device because it's a different reference:

var userRepo = new MobileUserRepository((ILog)null);
var deviceRepo = new MobileDeviceRepository((ILog)null);

IReadOnlyList<MobileUser> allUser = userRepo.GetAllMobileUsersWithDevices();
MobileUser user = allUser.First();

IReadOnlyList<MobileDeviceInfo> allDevices = deviceRepo.GetMobileDeviceInfos(user.VwdId, true);
MobileDeviceInfo device = allDevices.First();
bool contains = user.DeviceInfos.Contains(device);
bool anyEqual = user.DeviceInfos.Any(x => x.DeviceToken == device.DeviceToken);
Assert.IsTrue(contains); // no, it's false

The second approach with LINQ's Enumerable.Any returns the expected true.

If i don't use user.DeviceInfos.Contains(device) but user.DeviceInfos.ToList().Contains(device) it also works as expected since List<>.Contains uses my Equals.

The actual type of the ICollection<> seems to be a System.Collections.Generic.HashSet<MobileDeviceInfo> but if i use following code that uses also a HashSet<> it again works as expected:

bool contains = new HashSet<MobileDeviceInfo>(user.DeviceInfos).Contains(device); // true

So why are only references compared and my custom Equals is ignored?

Update:

even more confusing is the result is false even if i cast it to the HashSet<MobileDeviceInfo>:

 // still false
bool contains2 = ((HashSet<MobileDeviceInfo>)user.DeviceInfos).Contains(device);
// but this is true as already mentioned
bool contains3 = new HashSet<MobileDeviceInfo>(user.DeviceInfos).Contains(device); 

Update 2:: the reason for this really seems to be that both HashSets use different comparers. The entity-framework-HashSet uses a:

System.Data.Entity.Infrastructure.ObjectReferenceEqualityComparer

and the standard HashSet<> uses a:

GenericEqualityComparer<T>

That explains the issue, although i don't understand why entity framework uses an implementation that ignores custom Equals implementations under certain circumstances. That's a nasty trap, isn't it?


Conclusion: never use Contains if you don't know what comparer will be used or use Enumerable.Contains with the overload that takes a custom comparer:

bool contains = user.DeviceInfos.Contains(device, EqualityComparer<MobileDeviceInfo>.Default);  // true
like image 657
Tim Schmelter Avatar asked Aug 23 '16 13:08

Tim Schmelter


2 Answers

Why ICollection<>.Contains ignores my overridden Equals and the IEquatable<> interface?

Because there is no requirement from the implementors of the interface to do so.

ICollection<T>.Contains method MSDN documentation states:

Determines whether the ICollection<T> contains a specific value.

And then

Remarks

Implementations can vary in how they determine equality of objects; for example, List<T> uses Comparer<T>.Default, whereas Dictionary<TKey, TValue> allows the user to specify the IComparer<T> implementation to use for comparing keys.

Side note: Looks like they messed up IComparer<T> with IEqualityComparer<T>, but you get the point :)

Conclusion: never use Contains if you don't know what comparer will be used or use Enumerable.Contains with the overload that takes a custom comparer

According to the Enumerable.Contains<T>(IEnumerable<T>, T) method overload (i.e. without custom comparer) documentation:

Determines whether a sequence contains a specified element by using the default equality comparer.

which sounds like your overrides will be called. But then comes the following:

Remarks
If the type of source implements ICollection<T>, the Contains method in that implementation is invoked to obtain the result. Otherwise, this method determines whether source contains the specified element.

which conflicts with the initial statement.

It's really a mess. All I can say is that I fully agree with that conclusion!

like image 54
Ivan Stoev Avatar answered Nov 06 '22 23:11

Ivan Stoev


From the EF source, you might stumble on CreateCollectionCreateDelegate, which seems to be called as part of hooking up navigation properties.

This calls EntityUtil.DetermineCollectionType and returns a HashSet<T> as the type if that is compatible with the property.

Then, armed with HashSet<T>, it makes a call to DelegateFactory.GetNewExpressionForCollectionType which, per the code and the description, handles HashSet<T> as a special case and passes it an ObjectReferenceEqualityComparer in the constructor.

So: the HashSet<T> EF creates for you isn't using your equality implementation, it uses reference equality instead.

like image 31
Charles Mager Avatar answered Nov 06 '22 23:11

Charles Mager