List.Contains always giving false

Question

It seems that this problem has already been encountered by quite a few people:

List not working as expected

Contains always giving false

So I saw the answers and tried to implement the override of Equals and of GetHashCode but there seems that I am coding something wrong.

This is the situation: I have a list of Users(Class), each user has a List and a Name property, the list property contains licenses. I am trying to do a

         if (!users.Contains(currentUser))

but it is not working as expected. And this is the code I did to override the Equals and GetHashCode:

    public override bool Equals(object obj)
    {
        return Equals(obj as User);
    }


    public bool Equals(User otherUser)
    {
        if (ReferenceEquals(otherUser, null))
            return false;

        if (ReferenceEquals(this, otherUser))
            return true;

        return this._userName.Equals(otherUser.UserName) && 
               this._licenses.SequenceEqual<string>(otherUser.Licenses);
    }

    public override int GetHashCode()
    {
        int hash = 13;
        if (!_licenses.Any() && !_userName.Equals(""))
        {
            unchecked
            {
                foreach (string str in Licenses)
                {
                    hash *= 7;
                    if (str != null) hash = hash + str.GetHashCode();
                }
                hash = (hash * 7) + _userName.GetHashCode();
            }
        }
        return hash;
    }

thank you for your suggestions and help in advance!

EDIT 1:

this is the code where I am doing the List.Contains, I am trying to see if the list already contains certain user, if not then add the user that isn't there. The Contains only works the first time, when currentUser changes then the User inside the list changes to the current user maybe this is a problem that is unrelated to the equals, any ideas?

        if (isIn)
        {
            if (!listOfLicenses.Contains(items[3]))
                listOfLicenses.Add(items[3]);

            if (!users.Contains(currentUser))
            {
                User user2Add = new User();
                user2Add.UserName = currentUser.UserName;
                users.Add(user2Add);
                userIndexer++;
            }

            if (users[userIndexer - 1].UserName.Equals(currentUser.UserName))
            {
                users[userIndexer - 1].Licenses.Add(items[3]);
            }
            result.Rows.Add();
        }

Answer 1

Well, one problem with your hash code - if either there are no licences or the username is empty, you're ignoring the other component. I'd rewrite it as:

public override int GetHashCode()
{
    unchecked
    {
        int hash = 17;
        hash = hash * 31 + _userName.GetHashCode();
        foreach (string licence in Licences)
        {
            hash = hash * 31 + licences.GetHashCode();
        }
        return hash;
    }
}

Shorter and simpler. It doesn't matter if you use the hash code of the empty string, or if you iterate over an empty collection.

That said, I'd have expected the previous code to work anyway. Note that it's order sensitive for the licences... oh, and List<T> won't use GetHashCode anyway. (You should absolutely override it appropriately, but it won't be the cause of the error.)

It would really help if you could show a short but complete program demonstrating the problem - I strongly suspect that you'll find it's actually a problem with your test data.

Answer 2

After users[userIndexer - 1].Licenses.Add(items[3]) , users[userIndexer - 1] is not the same user anymore. You have changed the Licences which is used in equality comparison(in User.Equals).

--EDIT See below code

public class Class
{
    static void Main(string[] args)
    {
        User u1 = new User("1");
        User u2 = new User("1");
        Console.WriteLine(u1.Equals(u2));
        u2.Lic = "2";
        Console.WriteLine(u1.Equals(u2));
    }
}

public class User
{
    public string Lic;        
    public User(string lic)
    {
        this.Lic = lic;
    }

    public override bool Equals(object obj)
    {
        return (obj as User).Lic == Lic;
    }
}

Answer 3

You need you implement Equals and GetHashcode for the License class, otherwise SequenceEqual will not work.

Answer 4

Does your class implement IEquatable<User>? From your equality methods it appears it does but just checking.

The documentation for List.Contains states that:

This method determines equality by using the default equality comparer, as defined by the object's implementation of the IEquatable(T).Equals method for T (the type of values in the list)

Answer 5

It is very important to make sure that the value returned by GetHashCode never ever ever changes for a specific instance of an object. If the value changes then lists and dictionaries won't work correctly.

Think of GetHashCode as "GetPrimaryKey". You would not change the primary key of a user record in a database if someone added a new license to the user. Likewise you mustn't change the GetHashCode.

It appears from your code that you are changing the licenses collection and you're using that to calculate your hash code. So that is probably causing your issue.

Now, it is perfectly legitimate to use a constant value for every hash code you produce - you could just return 42 for every instance, for example. This will force calling Equals to determine if two objects are equal or not. All that having distinct hash codes does is short circuits the need to call Equals.

If the _userName field doesn't change then just return its hash code and see it that works.