Equal override

Dlaczego nie lubię override Equal? Wszystko jest dla ludzi. Pewnie się nawet zbłaźnię tą opowieścią :]
Ostatnio poszukiwałam błędu w kodzie, w którym przeciążone były funkcje Equals (o czym dowiedziałam się po znacznym czasie spędzonym z debuggerem), a czasem także ==. Błąd okazał się dość paskudny bo był nieprzewidywalny. Problem był po dodaniu elementu do kolekcji - nagle zwracane były nieprawidłowe elementy z kolekcji.
Na pierwszy rzut oka Equals wyglądał dobrze, zgodnie z zasadami zalecanymi przez msdn. Dodatkowo sprawdzana była zgodność referencji mniej więcej w tym tonie (pominęłam kwestie nulli):
    public class Person
    {
        private long _ID;
        public long ID
        { get { return _ID; } set { _ID = value; } }

        private string _Name;
        public string Name { get { return _Name; } set { _Name = value; } }

        public override bool Equals(object obj)
        {
            //obie alternatywy działają równie dobrze
            if (base.Equals(obj))//Object.ReferenceEquals(this, obj))
            {
                return true;
            }
            else
            {
                return this.Name == (obj as Person).Name;
            }
        }
    }
Wszystko będzie dobrze jeśli zadbamy o unikalność Name, w kodzie który poprawiałam unikalność nie była zachowana, dlatego wyniki były niezgodne z oczekiwaniami dla nowych elementów listy (w przeciwnym przypadku działał bazowy Equal porównujący referencje).
//troche danych testowych, ID zgadza się z indexem w liście.            
            Person expectedOla = new Person() { ID = 3, Name = "Ola" };
            Person expectedAla = new Person() { ID = 2,Name = "Ala" };
            List p =
            new List()
            {
                new Person() { ID = 0,Name = "Ala" },
                new Person() { ID = 1,Name = "Ola" },
                expectedAla,
                expectedOla
            };
var iola = p.IndexOf(expectedOla)
//iola == 1
Całkiem oczywisty wynik skoro porównujemy Name, no ale przecież porównujemy także referencje! Jednak kiedy referencja się nie zgadza wracamy do porównania Name. IndexOf jak pisze msdn działa od początku listy porównując każdy element - więc znajduje Olę pod 1. Tak naprawdę p.IndexOf(new Person()) po dekompilacji jakimś sprytnym narzędziem daje:
[System.Runtime.TargetedPatchingOptOutAttribute(@"Performance critical to inline across NGen image boundaries")]
public virtual int IndexOf(T item) {
  return Array.IndexOf(this._items, item, 0, this._size);
}
Na pewno używając jakieś funkcji warto było by dla przypomnienia sięgnąć do MSDNa. Ja w większości przypadków opieram się niestety na intelisens (choć ostatnio musiało się to zmienić) - mimo że w większości przypadków to wystarcza, jednak tam nie znajdziemy informacji że IndexOf bazuje na funkcji Equals, a jeśli jest ona przeciążona bazuje na jej bieżącym przeciążeniu.

Koniec końców musimy być uważni pisząc kod, a potem go naprawiając. Nie chodzi o to żeby fajnie coś się samo rzutowało, porównywało i inne bajery. Owszem jest to fajne, nawet w odkrywczym debugowaniu, jednak termin nie czeka na 'fajne. A nowa osoba w projekcie niekoniecznie będzie wiedzieć o jakimś przeciążeniu i mogą generować się błędy (oczywiście że błędy tak jak bałagan czy wiry w zlewie robią się same :]) bardzo pracochłonne w namierzeniu. Osobiście jestem za jakąś inną funkcją może CompareTo? ale na pewno taką którą łatwo się refaktoryzuje, wyszukuje i zmienia (a to przecież robi się ciągle i to często w kółko w zależności od klienta) przez cały system nie zastanawiając się czy dodatkowo jakaś funkcja .Netowa na niej nie bazuje.

Jednak nie jestem absolutnym przeciwnikiem. Bardzo przydatne jest przeciążenie Equals dla np słowników - obiektów w miarę rzadko zmieniających swoje wartości i na pewno rzadko zmieniających strukturę (słowniki zazwyczaj nie zmieniają się wiele przez całe życie projektu, przynajmniej tak wynika z mojego doświadczenia :] ). Przeciążenie Equals zagwarantuje nam możliwość łatwiejszego używania takich obiektów np w ComboBoxach.
Wszystko jest dla ludzi, ale - jak jeże - ostrożnie

Komentarze

  1. Nie wiem czy dokładnie przeczytałem, ale konkluzja wydaje się oczywista: to przeciążenie equal jest po prostu błędne - czemu nie są porównywane id, skoro występują w klasie? Bez sensu.

    OdpowiedzUsuń
  2. Przykład jest przejaskrawiony, w prawdziwej app były tylko pola stringowe, których ilość radośnie przyrastała a nie zawsze trafiała do Equals.

    OdpowiedzUsuń
  3. A podobno wiry w zlewie w Australii kręcą się w drugą stronę... więc też trzeba uważać ;-)

    OdpowiedzUsuń
  4. Praktycznie każdy programista miał z tymi funkcjami problem.

    Ja osobiście kiedyś równość encji oparłem o równość ID problem w tym że wszystkie encje które nie zostały jeszcze zapisane w bazie miały ID równe 0.

    Swojego czasu widziałem też kod w którym równość elementów była oparta na równości wartości zwracanej przez GetHashcode (co działało dopóki nie znalazł się przypadek że kilka różnych obiektów miało ten sam HashCode).

    OdpowiedzUsuń
  5. Ja od jakiegoś czasu stosuję podejście opisane pod
    tym adresem. Nie miałem jeszcze problemów.

    OdpowiedzUsuń

Publikowanie komentarza

Popularne posty