0

In the code block below I would expect dictCars to contain: { Chevy:Camaro, Dodge:Charger }

But, dictCars comes back empty. Because this line returns false each time it's called:

if(myCars.Contains(new Car(Convert.ToInt64(strCar.Split(':')[1]),strCar.Split(':')[2])))

Code block:

public class Car
{
    public long CarID { get; set; }

    public string CarName { get; set; }

    public Car(long CarID, string CarName)
    {
        this.CarID = CarID;
        this.CarName = CarName;
    }
}

List<Car> myCars = new List<Car>();
myCars.Add(new Car(0,"Pinto"));
myCars.Add(new Car(2,"Camaro"));
myCars.Add(new Car(3,"Charger"));

Dictionary<string, string> dictCars = new Dictionary<string, string>();
string strCars = "Ford:1:Mustang,Chevy:2:Camaro,Dodge:3:Charger";
String[] arrCars = strCars.Split(',');
foreach (string strCar in arrCars)
{
    if(myCars.Contains(new Car(Convert.ToInt64(strCar.Split(':')[1]),strCar.Split(':')[2])))
    {
        if (!dictCars.ContainsKey(strCar.Split(':')[0]))
        {
            dictCars.Add(strCar.Split(':')[0], strCar.Split(':')[2]);
        }
    }
}

return dictCars;

Question: What am I doing wrong with my List.Contains implementation?

Thanks in advance!

s15199d
  • 7,261
  • 11
  • 43
  • 70
  • Yeah, this is as you wrote `if (new Car(1,"car") == new Car(1,"car")) { ... }` and expected that the code `...` ever gets executed. Without implementation of special methods and operators this will never be true for reference types. – Paul Michalik Oct 11 '12 at 15:48

9 Answers9

8

You need to tell Contains what makes two Cars equal. By default it will use ReferenceEquals which will only call two objects equal if they are the same instance.

Either override Equals and GetHashCode in your Car class or define an IEqualityComparer<Car> class and pass that to Contains.

If two Cars that have the same CarID are "equal" then the implementation is pretty straightforward:

public override bool Equals(object o)
{
   if(o.GetType() != typeof(Car))
     return false;

   return (this.CarID == ((Car)o).CarID);
}

public override int GetHashCode()
{
  return CarID.GetHashCode();
}
D Stanley
  • 149,601
  • 11
  • 178
  • 240
6

Your Car class is a reference type. By default reference types are compared to each other by reference, meaning they are considered the same if they reference the same instance in memory. In your case you want them to be considered equal if they contain the same values.

To change the equality behavior, you need to override Equals and GetHashCode.

If two cars are equal only when ID and Name are equal, the following is one possible implementation of the equality members:

protected bool Equals(Car other)
{
    return CarID == other.CarID && string.Equals(CarName, other.CarName);
}

public override bool Equals(object obj)
{
    if (ReferenceEquals(null, obj))
        return false;
    if (ReferenceEquals(this, obj))
        return true;
    var other = obj as Car;
    return other != null && Equals(other);
}

public override int GetHashCode()
{
    unchecked
    {
        return (CarID.GetHashCode() * 397) ^ 
                (CarName != null ? CarName.GetHashCode() : 0);
    }
}

This implementation has been created automatically by ReSharper.
It takes into account null values and the possibility of sub-classes of Car. Additionally, it provides a useful implementation of GetHashCode.

Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
2

You are assuming that two Car instances that have the same CarID and CarName are equal.

This is incorrect. By default, each new Car(...) is different from each other car, since they are references to different objects.

There are a few ways to "fix" that:

  • Use a struct instead of a class for your Car.

    Structs inherit ValueType's default implementation of Equals, which compares all fields and properties to determine equality.

    Note that in this case, it is recommended that you make your Car struct immutable to avoid common problems with mutable structs.

  • Override Equals and GetHashCode.

    That way, List.Contains will know that you intend Cars with the same ID and Name to be equal.

  • Use another method instead of List.Contains.

    For example, Enumerable.Any allows you to specify a predicate that can be matched:

    bool exists = myCars.Any(car => car.ID == Convert.ToInt64(strCar.Split(':')[1])
                                    && car.Name = strCar.Split(':')[2]);
    
Community
  • 1
  • 1
Heinzi
  • 167,459
  • 57
  • 363
  • 519
2

You can add this code, by implementing IEquatable

public class Car: IEquatable<Car>
{

    ......

    public bool Equals( Car other )
    {
        return this.CarID  == other.CarID && this.CarName == other.CarName;
    }
}

Link : http://msdn.microsoft.com/fr-fr/library/vstudio/ms131187.aspx

Aghilas Yakoub
  • 28,516
  • 5
  • 46
  • 51
1

You need to implement the IEqualityComparer

More information on how to do it can be found here; http://msdn.microsoft.com/en-us/library/bb339118.aspx

// Custom comparer for the class 
 class CarComparer : IEqualityComparer<Car>
{
// Products are equal if their names and product numbers are equal. 
public bool Equals(Car x, Car y)
{

    //Check whether the compared objects reference the same data. 
    if (Object.ReferenceEquals(x, y)) return true;

    //Check whether any of the compared objects is null. 
    if (Object.ReferenceEquals(x, null) || Object.ReferenceEquals(y, null))
        return false;

    //Check whether the properties are equal. 
    return x.CarID == y.CarID && x.CarName == y.CarName;
}

// If Equals() returns true for a pair of objects  
// then GetHashCode() must return the same value for these objects. 

public int GetHashCode(Car car)
{
    //Check whether the object is null 
    if (Object.ReferenceEquals(car, null)) return 0;

    //Get hash code for the Name field if it is not null. 
    string hashCarName = car.CarName == null ? 0 : car.CarName.GetHashCode();

    //Get hash code for the ID field. 
    int hashCarID = car.CarID.GetHashCode();

    //Calculate the hash code for the product. 
    return hashCarName ^ hashCarID;
}

Check for equality;

CarComparer carComp = new CarComparer();
bool blnIsEqual = CarList1.Contains(CarList2, carComp);
A. Agius
  • 1,211
  • 1
  • 14
  • 28
1

You need to implement Equals. Most probably as:

public override bool Equals(object obj)
{
     Car car = obj as Car;
     if(car == null) return false;
     return car.CarID == this.CarID && car.CarName == this.CarName;
}
Nick Jones
  • 6,413
  • 2
  • 18
  • 18
  • @DanielHilgarth: I wasn't the one who downvoted, but it could be because [overriding only Equals but not GetHashCode is bad](http://stackoverflow.com/q/371328/87698) and can lead to unexpected problems. – Heinzi Oct 11 '12 at 16:15
0

Your car class needs to implement interface IEquatable and define an Equals method, otherwise the contains method is comparing the underlying references.

Mike Corcoran
  • 14,072
  • 4
  • 37
  • 49
0

A collection can never "contain" a newly newed object which uses the default Object.Equals comparison. (The default comparison is ReferenceEquals, which simply compares instances. This will never be true comparing an existing Car with a new Car())

To use Contains in this way, you will need to either:

  1. Override Car.Equals (and Car.GetHashCode) to specify what it means to be equivalent, or

  2. Implement an IEqualityComparer<Car> to compare the instances and specify that in your call to Contains.

Note the side effect that in the first option, other uses of Car.Equals(Car) will also use this comparison.


Otherwise, you can use Any and specify the comparison yourself (but IMHO this smells a little funny - a Car should know how to compare itself):

if(myCars.Any(c=> c.CarID == Convert.ToInt64(strCar.Split(':')[1]) && c.CarName == strCar.Split(':')[2]))
lc.
  • 113,939
  • 20
  • 158
  • 187
  • "Note the side effect that in the first option, other uses of Car == Car will also use this comparison.": That is not correct. If you don't explicitly override the equality operator, it will still perform a reference comparison. – Daniel Hilgarth Oct 11 '12 at 15:57
  • @DanielHilgarth You're right. Edited. You're supposed to read what I *mean* not what I type!! :-P – lc. Oct 11 '12 at 16:01
  • Hehe, alright. There is another error: You can't pass an `IEqualityComparer` to `List.Contains`. – Daniel Hilgarth Oct 11 '12 at 16:09
  • Aha, that's the one you want to use. You are right, that works, but that's not the `Contains` method in `List`. – Daniel Hilgarth Oct 11 '12 at 16:30
  • @DanielHilgarth Yup, but I didn't say `List.Contains`, I just said "Contains" (I think) :-D – lc. Oct 11 '12 at 16:32
-1
myCars.Contains(newCar)
myCars.Where(c =>  c.CarID == newCar.CarID && c.CarName==newCar.CarName).Count() > 0
Joe Mayo
  • 7,501
  • 7
  • 41
  • 60
Eduardo Aguiar
  • 247
  • 2
  • 3