List contains is always false C#

Question

I have a structure (struct) with 2 fiels ID and Name. Then I created a List of this structure but anytime I do a contains it works for the first time adding it to the collection but then it's not working anymore. Why? It's not a reference is a struct. I want to validate if this isn't in the list add it.

public struct MyCar
{
    public int id { get; set; }
    public string name { get; set; }
}

List<MyCar> cars = new List<MyCar>();
MyCar myCar = new MyCar();
myCar.id = 1;
myCar.name = "a";
if(cars.Contains(myCar) == false)                
{
    cars.Add(myCar);
}
myCar = new MyCar();
myCar.id = 2;
myCar.name = "b";
if(cars.Contains(myCar) == false)                
{
    cars.Add(myCar);
}
myCar = new MyCar(); //Wrong. Duplicate and it's gonna be added again because Contains == false
myCar.id = 1;
myCar.name = "a";
if(cars.Contains(myCar) == false)                
{
    cars.Add(myCar);
}

Maybe I can use the Find to match for => X.ID and => X.NAME but I don't want this because my struct in fact is more complex that this two fields.

Avoiding mutable value types would be a good first step - and if you want a particular kind of equality, you should specify that by overriding `Equals` and `GetHashCode`. — Jon Skeet, Nov 08 '13 at 17:06
"*... I don't want this because my struct in fact is more complex that this two fields.*" Are you sure you should be using a `struct` for this. Structs should be immutable and be small in size. Can you explain why you are using a `struct` instead of a `class`? — Scott Chamberlain, Nov 08 '13 at 17:16
I can't repeat the problem, there are only 2 cars added when running on 4.5. — Joachim Isaksson, Nov 08 '13 at 17:20
"... I don't want this because my struct in fact is more complex that this two fields." With the above example (since not all properties are being shown), this will probably end up working for anyone trying to test the given code. With the default behavior of structs, the first and third "MyCar" would be considered equal. — docmanhattan, Nov 08 '13 at 17:25

Scott Chamberlain · Accepted Answer · 2013-11-08T17:38:25.227

4

To use Contains class must override bool Equals(object obj) (and preferabley GetHashCode() too). Because you are using a struct instead of a class I would recomend implmenting IEquateable too;

public struct MyCar : IEquatable<MyCar>
{
    public int id { get; set; }
    public string name { get; set; }

    private static readonly StringComparer stringComparer = StringComparer.Ordinal;

    public override bool Equals(object obj)
    {
        if (obj is MyCar == false)
            return false;
        return Equals((MyCar)obj);
    }

    public bool Equals(MyCar car)
    {
        return this.id.Equals(car.id) && stringComparer.Equals(this.name,car.name);
    }

    public override int GetHashCode()
    {
        unchecked
        {
            int i = 17;
            i = i * 23 + id.GetHashCode();
            i = i * 23 + stringComparer.GetHashCode(name);
            return i;
        }
    }
}

edited Nov 08 '13 at 17:38

answered Nov 08 '13 at 17:08

Scott Chamberlain

124,994
33
282
431

I cannot do obj as MyCar even car == null. It's a structure and not a reference. – Maximus Decimus Nov 08 '13 at 17:13
@MaximusDecimus refresh the page, I caught that and have updated the code already. – Scott Chamberlain Nov 08 '13 at 17:14
Can I ask what does it means 17 and 23? – Maximus Decimus Nov 08 '13 at 17:23
Read the answer from [this stack overflow question](http://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-an-overridden-system-object-gethashcode), by multiplying and adding the numbers together like that you are more likely to get a "well distributed hash" than doing something simple like XORing the values or just adding them. 17 and 23 are chosen because they have been shown to be good factors to give you good hashes. – Scott Chamberlain Nov 08 '13 at 17:27
@MaximusDecimus I made a few small changes, the interface was spelled wrong and I set it up so now you can choose which string comparer to use in case you want to do something like a case insensitive comparison (just replace `StringComparer.Ordinal` with `StringComparer.OrdinalIgnoreCase`). – Scott Chamberlain Nov 08 '13 at 17:39

Mike Perrenoud · Answer 2 · 2013-11-08T17:37:21.287

Instead of using Contains you could use Any:

if (!cars.Any(m => m.id == myCar.id))

Now, if your heart is set on Contains, you'll need to implement IEquatable<MyCar> because Contains uses the default one; per MSDN Documentation:

This method determines equality by using the default equality comparer ...

public struct MyCar : IEquatable<MyCar>
{
    public int id { get; set; }
    public string name { get; set; }

    public bool Equals(MyCar other)
    {
        return this.id == other.id;
    }

    public override bool Equals(object obj)
    {
        if (!(obj is MyCar)) { return false; }
        return o.id == this.id;
    }

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

With that interface implemented you could now use Contains again. You need to think that through though because that means you're literally changing the default equality comparer for this type.

MyCar is a struct you can't do `obj as MyCar` nor `o == null`. (I made the same mistake in my answer but caught it) — Scott Chamberlain, Nov 08 '13 at 17:17

score 1 · Answer 3 · answered Nov 08 '13 at 17:19

I'd have expected the Equals operator to be the answer, but when i test in a console app in .NET 4, i don't even get the error condition - probably coz the default ValueType.Equals compares all fields anyways. So where are you running this to get this error?

        var a = new MyCar() { id = 1, name = "a" };
        var b = new MyCar() { id = 1, name = "a" };

        var x = new List<MyCar>();

        x.Add(a);

        Console.WriteLine(a.Equals(b));
        Console.WriteLine(x.Contains(a));
        Console.WriteLine(x.Contains(b)); //all 3 are true

List contains is always false C#

3 Answers3