KeyNotFoundException in C# Dictionary after changing property value based on what, the GetHashCode is calculated. Why?

Question

See the code below.

            static void Main(string[] args)
            {
    // Create Dictionary
                var dict = new Dictionary<TestClass, ValueClass>();

    // Add data to dictionary
                CreateSomeData(dict); 

    // Create a List
                var list = new List<TestClass>();
                foreach(var kv in dict) {
    // Swap property values for each Key
    // For example Key with property value 1 will become 6
    // and 6 will become 1
                    kv.Key.MyProperty = 6 - kv.Key.MyProperty + 1;

    // Add the Key to the List
                    list.Add(kv.Key);
                }

// Try to print dictionary and received KeyNotFoundException.
                foreach (var k in list)
                {
                    Console.WriteLine($"{dict[k].MyProperty} - {k.MyProperty}");
                }
            }



    static void CreateSomeData(Dictionary<TestClass, ValueClass> dictionary) {
        dictionary.Add(new TestClass {MyProperty = 1}, new ValueClass {MyProperty = 1});
        dictionary.Add(new TestClass {MyProperty = 2}, new ValueClass {MyProperty = 2});
        dictionary.Add(new TestClass {MyProperty = 3}, new ValueClass {MyProperty = 3});
        dictionary.Add(new TestClass {MyProperty = 4}, new ValueClass {MyProperty = 4});
        dictionary.Add(new TestClass {MyProperty = 5}, new ValueClass {MyProperty = 5});
        dictionary.Add(new TestClass {MyProperty = 6}, new ValueClass {MyProperty = 6});
    }

Key and Value Class:

namespace HashDictionaryTest
{
    public class TestClass
    {
        public int MyProperty { get; set; }

        public override int GetHashCode() {
            return MyProperty;
        }
    }

}

namespace HashDictionaryTest
{
    public class ValueClass
    {
        public int MyProperty { get; set; }

        public override int GetHashCode() {
            return MyProperty;
        }
    }

}

I am using the dotnet core 2.2 on Ubuntu. I did this test out of curiosity only. However, to my surprise, I got KeyNotFoundException.

I expected to receive the wrong values. However, I received the exception as mentioned above.

What I want to know is, why we got this error? What is the best practice in generating the HashCode so that we can avoid such issues?

It would be awesome if you could provide a [mcve]. Missing def for `ValueClass` — jazb, Feb 04 '19 at 01:56
Thank You @JohnB I added the complete code. Let me know if I am missing something. Also clarified my question. — Ripal Barot, Feb 04 '19 at 02:03
this is a weird bit of code you have. you're messing around with keys then expecting them to somehow magically be findable again? maybe explain what your goal is... — jazb, Feb 04 '19 at 02:08
The Dictionary does not support mutation of the keys https://stackoverflow.com/questions/3007296/c-sharp-dictionary-and-mutable-keys — AaronHolland, Feb 04 '19 at 02:08
try messing around with the `Value` property instead... not the `Key` — jazb, Feb 04 '19 at 02:10
@JohnB, I wanted to know two things. 1. Why it gave "KeyNotFound" vs giving me the wrong value? I get that now. 2. What is the best practice here if I want to write GetHashCode()? Due to the confidentiality agreement, I cannot reveal the real business requirements. — Ripal Barot, Feb 04 '19 at 02:17
read it, it is good article : https://www.codeproject.com/Tips/1255596/Overriding-Equals-GetHashCode-Laconically-in-CShar — Amit, Feb 04 '19 at 05:23
Actually supporting equality is a mess. In addition to `GetHashCode`, You'll also want to implement `IEquatable`, override `Equals(object)`, implement `Equals(TestClass)`, implement `operator==`, and implement `operator!=`. — Brian, Feb 07 '19 at 16:19

Eric Lippert · Answer 1 · 2019-02-04T22:29:00.680

5

What I want to know is, why we got this error?

There are guidelines for GetHashCode, and there are rules. If you violate the guidelines, you get lousy performance. If you violate the rules, things break.

You are violating the rules. One of the rules of GetHashCode is while an object is in a dictionary, its hash code must not change. Another rule is that equal objects must have equal hash codes.

You've violated the rules, and so things are broken. That's your fault; don't violate the rules.

What is the best practice in generating the HashCode so that we can avoid such issues?

For a list of the rules and guidelines, see:

https://ericlippert.com/2011/02/28/guidelines-and-rules-for-gethashcode/

edited Feb 04 '19 at 22:29

answered Feb 04 '19 at 22:22

Eric Lippert

647,829
179
1,238
2,067

This is what I needed. Thank You. – Ripal Barot Feb 05 '19 at 01:15
1

@RipalBarot: You're welcome! My goal for writing that article was to get all the rules in one place for easy consumption, because there are a lot of developers like you who don't know exactly what the rules are and wind up in trouble as a result. Glad I could help. – Eric Lippert Feb 05 '19 at 16:17

Hasan Emrah Süngü · Answer 2 · 2019-02-04T02:32:17.427

4

That is the expected behavior with your code. Then what is your wrong with your code?

Look at your Key Class. You are overriding your GetHashCode() and on top of that you are using a mutable value to calculate the GetHashCode() method (very very bad :( ).

public class TestClass
{
    public int MyProperty { get; set; }

    public override int GetHashCode() {
        return MyProperty;
    }
}

The look up in the implementation of the Dictionary uses GetHashCode() of the inserted object. At the time of insertion of your object your GetHashCode() returned some value and that object got inserted into some bucket. However, after you changed your MyProperty; GetHashCode() does not return the same value therefore it can not be look-ed up any more

This is where the lookup occurs

Console.WriteLine($"{dict[k].MyProperty} - {k.MyProperty}");

dict[k] already had its MyProperty changed therefore GetHashCode() does not return the value when the object first added to the dictionary.

Ant another really important thing is to keep in mind that when you override GetHashCode() then override Equals() as well. The inverse is true too!

edited Feb 04 '19 at 02:32

answered Feb 04 '19 at 02:05

Hasan Emrah Süngü

3,488
1
15
33

+1 for the answer. What is the best practice for GetHashCode()? How would you generate the hashcode in such scenario? – Ripal Barot Feb 04 '19 at 02:10
`GetHashCode()` is not supposed to use mutable data to calculate hash. In your case you can make `MyProperty` into immutable. – Hasan Emrah Süngü Feb 04 '19 at 02:11
2

@HasanEmrahSüngü He is not using a mutable type for hashcode. `int` is immutable. The problem is that a property taking part in `GetHashCode` is not expected to change over the lifecycle of the object. – kovac Feb 04 '19 at 02:17
@swdon, I would check the definition of mutable in your case :) He is using some `int` value at the insertion time then magically that `int` value is `changed` into another thing. Keyword being change. By the way where in my answer did I use `mutable type`? – Hasan Emrah Süngü Feb 04 '19 at 02:18
I guess what you meant is along the lines of remove the setter for `MyProperty`, set `MyProperty` in the constructor and then override `GetHashCode()` like `return MyProperty.GetHashCode()`. – kovac Feb 04 '19 at 02:20
4

@swdon, What I meant is do not use values that are subject to change while calculating `GetHashCode()` – Hasan Emrah Süngü Feb 04 '19 at 02:21
@radarbob, I am not sure if you are commenting on my answer? Or some other users deleted comments? – Hasan Emrah Süngü Feb 04 '19 at 02:56
@HasanEmrahSüngü, sorry for the confusion. I removed my comments into an answer – radarbob Feb 04 '19 at 04:13

radarbob · Accepted Answer · 2019-02-04T22:55:32.763

KeyNotFoundException ... Why?

The distilled, core reason is that the Equals and GetHashCode methods are inconsistent. This situation is fixed by doing 2 things:

Override Equals in TestClass
Never modify a dictionary during iteration
- It's that the key object/value is being modified

GetHashCode - Equals disconnect

... GetHashCode is not expected to change over the lifecycle of the object. AND do not use values that are subject to change while calculating GetHashCode()
- Not exactly. The problem is a changing hash code while equality does not change.
- MSDN says: The GetHashCode() method for an object must consistently return the same hash code as long as there is no modification to the object state that determines the return value of the object's System.Object.Equals method

TestClass.Equals

I say TestClass because that is the dictionary key. But this applies to ValueClass too.

A class' Equals and GetHashCode must be consistent. When overriding either but not both then they are not consistent. We all know "if you override Equals also override GetHashCode". We never override GetHashCode yet seem to get away with it. Hear me now and believe me the first time you implement IEqualityComparer and IEquatable - always override both.

Iterating Dictionary

Do not add or delete an element, modify a key, nor modify a value (sometimes) during iteration.

Editing Dictionary Values ... in a loop - It hits on some technical reasons a dictionary entry cannot safely be modified during iteration
MSDN Dictionary doc
Can change value during iteration - The dictionary "value", not the key's value
Well, not in every case

GetHashCode

MSDN GetHashCode
- Do not use the hash code as the key to retrieve an object from a keyed collection.
- Do not test for equality of hash codes to determine whether two objects are equal

The OP code may not be doing this literally but certainly virtually because there is no Equals override.

Here is a neat hash algorithm from C# demi god Eric Lipper

KeyNotFoundException in C# Dictionary after changing property value based on what, the GetHashCode is calculated. Why?

3 Answers3