Implementing IEquatable<T> in a mutable type

Question

I have a class that represents an external physical measuring device. The simplified version looks like this:

public class Device {
    public string Tag { get; set; }
    public int Address { get; set; }
}

Tag is a user-defined value for identifying the device. Address is the value used by an adapter to communicate with the device. If two instances of Device have the same Address, then the same external measuring device will be used.

I'd like to mimic that behavior in code (for using methods like Contains and Distinct) by overriding Equals and implementing IEquatable<T>:

public class Device : IEquatable<Device> {
    public string Tag { get; set; }
    public int Address { get; set; }

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

As you can see, I'm ignoring the Tag property in the implementation of Equals.

So, my question is: Should I ignore the Tag property in the implementation of Equals? Does doing so make the code harder to understand? Is there a better way of doing what I'm trying to do? I need the Tag property because, often, the user will not know the Address, or even whether or not the Device has an Address (that is taken care of in the App.config file, and the user will be dealing with an interface called IDevice which doesn't have an Address property).

Update:

Thanks everyone for the responses.

So, I gather that I should be using a custom IEqualityComparer. Do you have any guidance on how to do so if my real code looks more like this?

public interface IDevice {
    string Tag { get; set; }
    double TakeMeasurement();
}
internal class Device : IDevice {
    public string Tag { get; set; }
    public int Address { get; set; }
    public double TakeMeasurement() {
        // Take a measurement at the device's address...
    }
}

Should I check the device type in my IEqualityComparer?

public class DeviceEqualityComparer : IEqualityComparer<IDevice> {
    public bool Equals(IDevice x, IDevice y) {
        Contract.Requires(x != null);
        Contract.Requires(y != null);
        if ((x is Device) && (y is Device)) {
            return x.Address.Equals(y.Address);
        }
        else {
            return x.Equals(y);
        }
    }

    public int GetHashCode(IDevice obj) {
        Contract.Requires(obj != null);
        if (obj is Device) {
            return obj.Address.GetHashCode();
        }
        else {
            return obj.GetHashCode();
        }
    }
}

Answer 1

First of all you forgot to override GetHashCode(), so your code is broken.

IMO you should override Equals only if two objects are equivalent for all purposes. And objects with different Tags seem different for some purposes.

I wouldn't override Equals on these objects at all. I'd rather implement a custom comparer IEqualityComparer<T> and use that where appropriate. Most methods that have a notion of equality, take an IEqualityComparer<T> as optional parameter.

I wouldn't forbid null parameters, but handle them. I've also added an early-out for referential equality.

public class DeviceByAddressEqualityComparer : IEqualityComparer<IDevice> {
    public bool Equals(IDevice x, IDevice y) {
        if(x==y)
          return true;
        if(x==null||y==null)
          return false;
        return x.Address.Equals(y.Address);
    }

    public int GetHashCode(IDevice obj) {
        if(obj == null)
          return 0;
        else
          return obj.Address.GetHashCode();
    }
}

If you want to check the type, depends on context. When overriding Equals I typically check with if x.GetType()==y.GetType(), but since you're using a special purpose comparer here, which deliberately ignores part of the object, I probably wouldn't make the type part of the identity.

Answer 2

Yes, your current implementation is definitely confusing. The equality you've defined is clearly not the right notion of equality for devices.

So, rather than implementing IEquatable<Device> as you've done, I'd define an implementation of IEqualityComparer<Device>, maybe

class DeviceAddressEqualityComparer : IEqualityComparer<Device> {
    public bool Equals(Device x, Device y) {
        Contract.Requires(x != null);
        Contract.Requires(y != null);
        return x.Address.Equals(y.Address);
    }

    public int GetHashCode(Device obj) {
        Contract.Requires(obj != null);
        return obj.Address.GetHashCode();
    }
}

You can pass instances of IEqualityComparer<T> to Contains, Distinct and other LINQ methods that depend on equality (e.g., GroupBy).