CODEWITHSUNDEEP
c#linqlistgenericscollections
Justin Homes
Justin Homes

Reputation: 3799

Distinct of List <T> not workiong

I have this class with comparer

public partial class CityCountryID :IEqualityComparer<CityCountryID>
{

    public string City { get; set; }
    public string CountryId { get; set; }

    public bool Equals(CityCountryID left, CityCountryID right)
    {
        if ((object)left == null && (object)right == null)
        {
            return true;
        }
        if ((object)left == null || (object)right == null)
        {
            return false;
        }
        return left.City.Trim().TrimEnd('\r', '\n') == right.City.Trim().TrimEnd('\r', '\n') 
            && left.CountryId == right.CountryId;
    }

    public int GetHashCode(CityCountryID obj)
    {
        return (obj.City + obj.CountryId).GetHashCode();
    }
}

I Tried using Hashset and Distinct but neither one is working. i did not want to do this in db as the list was too big and too for everrrrrrrr. why is this not working in c#? i want to get a unique country, city list.

            List<CityCountryID> CityList = LoadData("GetCityList").ToList();
            //var unique = new HashSet<CityCountryID>(CityList);
            Console.WriteLine("Loading Completed/ Checking Duplicates");
            List<CityCountryID> unique = CityList.Distinct().ToList();

Upvotes: 0

Views: 154

Answers (2)

Scott Chamberlain
Scott Chamberlain

Reputation: 127563

You needed to implment the interface IEquatable<T> not IEqualityComparer<T> (Be sure to read the documentation, especially the "Notes to Implementers" section!). IEqualityComparer is when you want to use a custom comparer other than the default one built in to the class.

Also you need to make the changes that Jon mentioned about GetHashCode not matching Equals

Upvotes: 3

Jon Skeet
Jon Skeet

Reputation: 1500785

Your Equals and GetHashCode methods aren't consistent. In Equals, you're trimming the city name - but in GetHashCode you're not. That means two equal values can have different hash codes, violating the normal contract.

That's the first thing to fix. I would suggest trimming the city names in the database itself for sanity, and then removing the Trim operations in your Equality check. That'll make things a lot simpler.

The second is work out why it was taking a long time in the database: I'd strongly expect it to perform better in the database than locally, especially if you have indexes on the two fields.

The next is to consider making your type immutable if at all possible. It's generally a bad idea to allow mutable properties of an object to affect equality; if you change an equality-sensitive property of an object after using it as a key in a dictionary (or after adding it to a HashSet) you may well find that you can't retrieve it again, even using the exact same reference.

EDIT: Also, as Scott noted, you either need to pass in an IEqualityComparer to perform the equality comparison or make your type override the normal Equals and GetHashCode methods. At the moment you're half way between the two (implementing IEqualityComparer<T>, but not actually providing a comparer as an argument to Distinct or the HashSet constructor). In general it's unusual for a type to implement IEqualityComparer for itself. Basically you either implement a "natural" equality check in the type or you implement a standalone equality check in a type implementing IEqualityComparer<T>. You don't have to implement IEquatable<T> - just overriding the normal Equals(object) method will work - but it's generally a good idea to implement IEquatable<T> at the same time.

As an aside, I would also suggest computing a hash code without using string concatenation. For example:

public override int GetHashCode()
{
    int hash = 17;
    hash = hash * 31 + CountryId.GetHashCode();
    hash = hash * 31 + City.GetHashCode();
    return hash;
}

Upvotes: 5

Related Questions