Have I implemented Equals()/GetHashCode() correctly?

Question

The program was working with this implementation:

class Instrument
{
    public string ClassCode { get; set; }
    public string Ticker { get; set; }
    public override string ToString()
    {
        return " ClassCode: " + ClassCode + " Ticker: " + Ticker + '.';
    }
}

But because I need to use Instrument in Dictionary I've decided to implement equals/hashcode:

class Instrument
{
    public string ClassCode { get; set; }
    public string Ticker { get; set; }
    public override string ToString()
    {
        return " ClassCode: " + ClassCode + " Ticker: " + Ticker + '.';
    }

    public override bool Equals(object obj)
    {
        if (obj == null)
            return false;

        Instrument instrument = obj as Instrument;
        if (instrument == null)
            return false;

        return ((ClassCode.Equals(instrument.ClassCode)) && (Ticker.Equals(instrument.Ticker));
    }

    public override int GetHashCode()
    {
        int hash = 13;
        hash = (hash * 7) + ClassCode.GetHashCode();
        hash = (hash * 7) + Ticker.GetHashCode();
        return hash;
    }
}

Now the program has stopped working. In such or similar places I receive "KeyNotFoundException":

if (cache.Keys.Any(instrument => instrument.Ticker == newTicker && instrument.ClassCode == newClassCode))

Is it possible that some pieces of the code assume that equals and hashcode IS NOT implemented? Or probably I just implemented them wrong? Sorry I'm not familiar with such advanced features in C# as the last piece of code and don't know how it is connected with equals or hashCode.

Answer 1

Your HashCode and Equals methods should depend only on immutable properties - your implementation uses ClassCode and Ticker which both have setters and are therefore mutable.

Answer 2

First, instead of using cache.Keys.Any you can just use ContainsKey.

bool contains = cache.ContainsKey(
    new Instrument { Ticker = newTicker, ClassCode = newClassCode });

The first iterate over the whole keys list - O(n), while the second uses Dictionary's built in hash table implementation - O(1).

Second, check for null reference in your implementation:

public override bool Equals(object obj)
{
    if (obj == null)
        return false;

    Instrument instrument = obj as Instrument;
    if (instrument == null)
        return false;

    // 1. string.Equals can handle null references.
    // 2. object.ReferenceEquals for better preformances when it's the same object
    return (object.ReferenceEquals(this, instrument)) ||
        (string.Equals(ClassCode, instrument.ClassCode) &&
        string.Equals(Ticker, instrument.Ticker));
}

public override int GetHashCode()
{
    int hash = 13;
    if (ClassCode != null)
        hash = (hash * 7) + ClassCode.GetHashCode();
    if (Ticker!= null)
        hash = (hash * 7) + Ticker.GetHashCode();

    return hash;
}

Other than that, I can't see a problem.