5

Working in Visual Studio 2008 (C#)... I use a List collection to store instances of my custom class (Shift).

I want to delete a certain shift from the list by using the Remove method.

But List.Remove() always deletes the first item it finds.

I've implemented the IComparable interface for my Shift, I thought this would be enough, then I added an implementation of IEqualityComparer, and it still has no effect.

Here's the excerpt with my implementation:

region IComparable Members

    public int CompareTo(object obj)
    {
        Shift s1 = this;
        Shift s2 = (Shift)obj;
        if (s1.start.time != s2.start.time)
            return s1.start.CompareTo(s2.start);
        else
            return s1.end.CompareTo(s2.end);
    }

endregion

region IEqualityComparer Members

    public bool Equals(Shift x, Shift y)
    {
        
        if ((x.opening) != (y.opening)) return false;
        if ((x.closing) != (y.closing)) return false;
        if (!x.opening) if (x._start != y._start) return false;
        if (!x.closing) if (x._end != y._end) return false;
        if (x.when != y.when) return false;
        if (x.day != y.day) return false;
        if (x.EmployeeID != y.EmployeeID) return false;
        return true;
    }

    public int GetHashCode(Shift obj)
    {
        return obj.ToString().ToLower().GetHashCode();
    }

endregion

And yet, still - when the List contains two shifts, say "8:00 - 15:00"; "12:00 - 16:00", calling Remove("12:00-16:00") results in "8:00 - 15:00" getting removed, and the latter one remains in the collection!

What's wrong here? Thx

Community
  • 1
  • 1
Konrad Morawski
  • 8,307
  • 7
  • 53
  • 91

4 Answers4

10

You can override object.GetHashCode and object.Equals:

public override bool Equals(object obj)
{
    if(obj == null)
    {
        return false;
    }
    return Equals(this, obj as Shift);
}

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

You should also probably do a null check in Equals(x, y).

Rex M
  • 142,167
  • 33
  • 283
  • 313
  • You should also return false if obj.GetType() != GetType(), otherwise you risk doing equality checks against subclasses which may have more refined algorithms. – Bryan Watts Sep 16 '09 at 18:12
  • -1, He did not overload object.GetHashCode(), he implemented IEqualityComparer.GetHashCode(T). – csharptest.net Sep 16 '09 at 18:22
  • BTW, I think he is simply misunderstanding the behavior of List.Remove() expecting it to perform a List.RemoveAll() (see my answer below). – csharptest.net Sep 16 '09 at 18:40
  • OIC, I missed that THX. It sounded like it was removing a 'first' one of several he wanted removed. *confused* I'm not sure how it was removing anything if he was using the default reference equality. Anyway sounds like he's working now. – csharptest.net Sep 16 '09 at 18:53
4

IComparable is not normally used to compare for equality (it's used for ordering), so List<T>.Remove() ignores it.

IEqualityComparer is not an equivalent of IComparable for equality purposes. It is supposed to be implemented by a comparer object - that is, an object that compares other objects for equality. If you want equality comparisons to be inherent to your class, then you rather need to implement IEquatable<T>. Or just override Object.Equals() and Object.GetHashCode() on your class, without implementing any interfaces.

Pavel Minaev
  • 99,783
  • 25
  • 219
  • 289
1

Remove uses EqualityComparer<T>.Default to determine equality and choose which object to remove, which will use IEquatable<T> if it's implemented on your object, otherwise, it will use reference equality.

You have two options to get the behavior you want:

1) Make Shift implement IEquatable<T> (not just override Object.Equals or make the method, but make Shift - Shift : IEquatable<Shift>)

2) Use List<T>.RemoveAt

Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
0

With the example you provided, you're calling:

List<Shift>.Remove("12:00 - 16:00");

"12:00 - 16:00" in this case, is a String value and not an actual Shift object. Make sure that in your CompareTo method that you're code is properly casting the String value to a Shift object. Otherwise, when it compares start times...things could go haywire.

Justin Niessner
  • 242,243
  • 40
  • 408
  • 536
  • Justin, as a matter of fact I don't really pass a string parameter, this was sort of "pseudo-code" on my part just to show the concept. Sorry if that was ambiguous – Konrad Morawski Sep 16 '09 at 18:34