C# - List<T>.Remove() always deletes the first object on the list

Question

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

Polyzoan

Answer 1

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).

Hugely

You should also return false if obj.GetType() != GetType(), otherwise you risk doing equality checks against subclasses which may have more refined algorithms. – Beading

-1, He did not overload object.GetHashCode(), he implemented IEqualityComparer<T>.GetHashCode(T). – Contrastive

BTW, I think he is simply misunderstanding the behavior of List.Remove() expecting it to perform a List.RemoveAll() (see my answer below). – Contrastive

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. – Contrastive

Answer 2

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.

Kendra

Actually Object.Equals() works just right! That seems like the simpliest option of all. Thank you – Polyzoan

Answer 3

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

Naca

Answer 4

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.

Nacreous

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 – Polyzoan