Правильно ли я реализовал Equals () / GetHashCode ()? - PullRequest
Новая
       17

Правильно ли я реализовал Equals () / GetHashCode ()?

8 голосов
/ 18 апреля 2011

Программа работала с этой реализацией: 

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

Но так как мне нужно использовать инструмент в словаре, я решил реализовать 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;
    }
}

Теперь программа перестала работать. В таких или похожих местах я получаю «KeyNotFoundException»: 

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

Возможно ли, что некоторые части кода предполагают, что равно и хеш-код НЕ реализован? Или, возможно, я просто неправильно их реализовал? Извините, я не знаком с такими расширенными функциями C # как последний кусок кода и не знаю, как он связан с equals или hashCode.

Ответы [ 3 ]

7 голосов
/ 18 апреля 2011

Ваши методы HashCode и Equals должны зависеть только от неизменных свойств - ваша реализация использует ClassCode и Ticker, которые оба имеют установщики и, следовательно, являются изменяемыми.

3 голосов
/ 18 апреля 2011

Первый , вместо использования cache.Keys.Any вы можете просто использовать ContainsKey . 

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

Первая итерация по всему списку ключей - O (n), а вторая использует реализацию встроенной в хэш-таблицу словаря - O (1).

Второй , проверьте нулевую ссылку в вашей реализации: 

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;
}

Кроме этого, я не вижу проблемы.

1 голос
/ 18 апреля 2011

Но так как мне нужно использовать инструмент в словаре, я решил реализовать equals / hashcode

Это неправильная причина. В вашем классе уже есть реализации Equality и GetHashCode, которые подходят, эффективны и протестированы для использования в Словаре.

Правильно ли я реализовал Equals () / GetHashCode ()?

Нет. Вам не хватает перегрузки для == для начала. И это будет надежно только тогда, когда вы сделаете Инструмент неизменным.

Ваш лучший способ действий - , а не , чтобы переопределить любого из этих членов.

Также см. этот совет MSDN . Обратите внимание на «гарантии равных» и

Переопределение оператора == в неизменяемых типах не рекомендуется.

...