-1

While comparing instances of a custom class, I noticed that a call to Contains doesn't work the way I expect it to. Assuming that the default comparison goes by the reference (pointer or whatever it's called), I implemented both CompareTo and Equals. I made sure to be implementing IComparable, of course.

It's still doesn't work and I get no hits when I put breakpoints on those methods.

What can I be missing and is the best option to use extension methods if I'm not?

public override bool Equals(Object input)
{
  return Id == ((MyType) input).Id;
}

public int CompareTo(Object input)
{
  return Id - ((MyType)input).Id;
}
  • 6
    You might need to override `GetHashCode()` and possibly `==` too. Which `Contains` are we talking about? The extension method in Linq? The one in List? – Cameron Jan 19 '15 at 19:34
  • What class are you are calling `Contains` from? – Steve Mitcham Jan 19 '15 at 19:35
  • Also, information on overriding Equals. http://msdn.microsoft.com/en-us/library/ms173147%28v=vs.80%29.aspx – Steve Mitcham Jan 19 '15 at 19:36
  • @Cameron Hmm... How do I know if it's LINQ's or List's? –  Jan 19 '15 at 19:36
  • @SteveMitcham From `List temp = ...;` –  Jan 19 '15 at 19:37
  • There's not really any magic or so going on, it largely depends on how your class you call .Contains(..) on implements that method. For List e.g. see http://stackoverflow.com/a/9264597/2591 – Jörg Battermann Jan 19 '15 at 19:38
  • @JörgB. I can't see the difference between that post's answer and what I'm trying... I overrode `Equals` but it's not being called. What am I missing? –  Jan 19 '15 at 19:41
  • @Andy J - it is hard to say what you are missing without seeing more of your code. The Equals method should be called if you call .Contains on the List so if that is not happening, you are doing something else incorrectly. However it is not possible for us to help you further with just the information you have provided. – user469104 Jan 19 '15 at 19:44
  • @Cameron Post your comment. That was it. And to me it's widely weird. When I override `GetHashCode`, the stuff work **but** it never enters it! It **only** enters `Equals` but that's **already** there... Unexpected indeed... –  Jan 19 '15 at 19:44
  • @user469104 Wrong. It's possible. And Cameron already spotted it. Please see my comment. Weirdness ahead. –  Jan 19 '15 at 19:45
  • 1
    @AndyJ if your MyType class implements IEquatable, you still "must" override GetHashCode() as most lookups are against instance's hashcodes' first, see http://blogs.msdn.com/b/jaredpar/archive/2009/01/15/if-you-implement-iequatable-t-you-still-must-override-object-s-equals-and-gethashcode.aspx . Give it a try - override .GetHashcode() { return base.GetHashCode(); } and place a break point in there and see if it gets hit. – Jörg Battermann Jan 19 '15 at 19:45
  • @JörgB. I only implement `IComparable`. Also, yes, as Cameron suggested, the thing works since I added `GetHashCode`. But it's never entering the method! –  Jan 19 '15 at 19:47
  • Code for List here shows that you should be overriding == as well as Equals as Cameron said http://referencesource.microsoft.com/#mscorlib/system/collections/generic/list.cs,cf7f4095e4de7646 – Steve Mitcham Jan 19 '15 at 19:47
  • Someone please put that as a reply so I can accept as an answer. –  Jan 19 '15 at 19:49
  • 2
    GetHashCode is a way to pre-check if things are equal without performing the whole compare. Two things with the same hash code are supposed to be equals. Theoretically, the hashcode is supposed to be a faster compare than the whole object but most people don't implement it correctly. – Steve Mitcham Jan 19 '15 at 19:49

1 Answers1

2

A better implementation could be:

public bool Equals(MyType other)
{
    // if 'other' is a null reference, or if 'other' is more derived or less derived
    if ((object)other == (object)null || other.GetType() != GetType())
        return false;

    // OK, check members (assuming 'Id' has a type that makes '==' a wise choice)
    return Id == other.Id;
}

public override bool Equals(object obj)
{
    // call to other overload
    return Equals(obj as MyType);
}

public override int GetHashCode()
{
    return Id.GetHashCode();
}

You can mark the class as implementing IEquatable<MyType> in that case (but it will work even without that).

Regarding GetHashCode: Always remember to override it. You should have seen a compiler warning that it was problematic to override Equals(object) without overriding GetHashCode. Never keep the code return base.GetHashCode() in the override (assuming the base class is System.Object). Either give it a try and implement something based on the members that participate in Equals. If you do not think GetHashCode will actually be used in your case, say:

public override int GetHashCode()
{
    throw new NotSupportedException("We don't have GetHashCode, sorry");
}

If you absolutely know that you will only be using List<>.Contains, and not e.g. Dictionary<,>, HashSet<> and not Linq's Distinct(), etc. etc., it could work with GetHashCode() simply throwing.

IComparable<MyType> is not needed unless you sort List<MyType> or MyType[], or you use Linq's OrderBy with MyType, or you use SortedDictionary<,>, SortedSet<>.

Overloading operator == is not needed for these uses.

Jeppe Stig Nielsen
  • 60,409
  • 11
  • 110
  • 181