в IEquatable <T>реализация необходима проверка ссылок - PullRequest
3 голосов
/ 13 января 2011

У меня есть класс, который внедряет IEquatable<T>. Нужно ли делать проверку ссылок в Equals() или это выполняется в рамках?

class Foo : IEquatable<Foo>
{
    int value;
    Bar system;

    bool Equals(Foo other)
    {
        return this == other || 
        ( value == other.value && system.Equals(other.system) );
    }
}

В приведенном выше примере выражение this==other является излишним или необходимым?

Обновление 1

Я понимаю, что мне нужно исправить код следующим образом:

    bool Equals(Foo other)
    {
        if( other==null ) { return false; }
        if( object.ReferenceEquals(this, other) ) { return true; } //avoid recursion
        return value == other.value && system.Equals(other.system);
    }

Спасибо за ответы.

Ответы [ 6 ]

4 голосов
/ 13 января 2011

Будь осторожен. На самом деле я бы настоятельно не одобрял этого, поскольку, если вы когда-нибудь захотите перегрузить оператор == для вашего типа Foo в терминах Equals (как это обычно делается в моем опыте), вы окажетесь сами с бесконечной рекурсией.

Чтобы проиллюстрировать, что я имею в виду, вот общая реализация == в терминах Equals:

public static bool operator ==(Foo x, Foo y)
{
    // Will overflow the stack if Equals uses ==:
    return !ReferenceEquals(x, null) && x.Equals(y);
}

Тем не менее, я могу искренне согласиться с замечанием Джона о том, что может быть целесообразно использовать ReferenceEquals вместо этого.

4 голосов
/ 13 января 2011

Обычно это оптимизация - это была бы странная реализация Equals, которая без нее потерпела бы неудачу.Поэтому я бы не расценил бы это как необходимое - но при этом он не "позаботился об этом в рамках".Это дешевая оптимизация, поэтому обычно стоит включить.

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

1 голос
/ 13 января 2011

Это не необходимо в том смысле, что оно может не требоваться для корректности, но структура, конечно, не "заботится об этом", поэтому может быть полезным дляв, как правило, из соображений производительности.

Одно замечание: если реализация обернута EqualityComparer<T>.Default, она не вводит код пользователя, если один или оба аргумента равны null, поэтому в этом случае она выполняет некоторую меру ссылкипроверка (если не полная ReferenceEquals(x, y)).

public override bool Equals(T x, T y)
{
    if (x != null)
    {
        return ((y != null) && x.Equals(y));
    }
    if (y != null)
    {
        return false;
    }

    return true;
}

Не по теме, в вашем примере метода есть несколько проблем с разыменованием (other может быть null, this.system может бытьnull).

Я бы написал ваш метод примерно так:

public bool Equals(Foo other)
{
    if(other == null)
         return false;

    if(other == this)
         return true;

    return value == other.value
            && EqualityComparer<Bar>.Default.Equals(bar, other.bar)
            // You don't *have to* go down the EqualityComparer route
            // but you do need to make sure you don't dereference null.

} ​​

Также не забудьте переопределить GetHashCode всякий раз, когда вы пишете свое собственное равенство.сравнения.

0 голосов
/ 13 января 2011

Я считаю, что это необходимо. Проверка ссылок - это первый быстрый шаг, который вы можете выполнить при сравнении объектов.

В приведенном вами примере обязательно проверьте, что other не равно нулю, прежде чем получить доступ к его значениям.

bool Equals(Foo other)
{
    if(other == null) return false;

    if(this == other) return true;

    // Custom comparison logic here.
}
0 голосов
/ 13 января 2011

IEquatable<T> - интерфейс; реализация зависит от исполнителя.

Поскольку вы реализуете интерфейс, вы несете ответственность за все ожидаемое поведение, определенное интерфейсом.

0 голосов
/ 13 января 2011

Я думаю, что необходимо проверить this == other, потому что вы определяете свой equals. Если вы не хотите, чтобы он проверялся указателем, вы пишете свой собственный IEquatable.

...