Czy taki kod to „wrak pociągu” (z naruszeniem Prawa Demeter)?

23

Przeglądając napisany przeze mnie kod, natknąłem się na następującą konstrukcję, która skłoniła mnie do myślenia. Na pierwszy rzut oka wydaje się wystarczająco czysty. Tak, w rzeczywistym kodzie getLocation()metoda ma nieco bardziej konkretną nazwę, która lepiej opisuje dokładnie, jaką lokalizację otrzymuje.

service.setLocation(this.configuration.getLocation().toString());

W tym przypadku servicejest to zmienna instancji znanego typu, zadeklarowana w ramach metody. this.configurationpochodzi z przekazania do konstruktora klasy i jest instancją klasy implementującej określony interfejs (która nakazuje getLocation()metodę publiczną ). Dlatego this.configuration.getLocation()znany jest typ zwracanego wyrażenia ; szczególnie w tym przypadku jest to java.net.URL, podczas gdy service.setLocation()chce String. Ponieważ oba typy String i URL nie są bezpośrednio kompatybilne, wymagana jest pewna konwersja, aby dopasować kwadratowy kołek do okrągłego otworu.

Jednakże , zgodnie z prawem Demetry jak cytowane w czystego kodu , metoda A F w klasie C powinien wywołać tylko metody, na C , przedmiotów wytworzonych lub przekazywane jako argumenty f i obiektów odbywa się w zmiennych instancji C . Cokolwiek poza tym (ostatni toString()w moim szczególnym przypadku powyżej, chyba że weźmiesz pod uwagę obiekt tymczasowy stworzony w wyniku samej inwokacji metody, w którym to przypadku całe Prawo wydaje się być dyskusyjne) jest niedozwolone.

Czy istnieje uzasadnione powód, dla którego połączenie takie jak powyższe, biorąc pod uwagę wymienione ograniczenia, powinno być zniechęcone, a nawet zabronione? A może po prostu jestem zbyt złośliwy?

Gdybym wdrożyć metodę URLToString(), która po prostu wywołuje toString()na URLobiekt (taki jak ten zwrócony przez getLocation()), przekazanym mu jako parametr i zwraca wynik, mogę zawinąć getLocation()wywołanie w to, aby osiągnąć dokładnie ten sam wynik; skutecznie przesunąłbym konwersję o jeden krok na zewnątrz. Czy to w jakiś sposób sprawi, że będzie to do przyjęcia? ( Wydaje mi się intuicyjnie, że nie powinno to robić żadnej różnicy, ponieważ wszystko to trochę przesuwa. Jednak, zgodnie z cytowaną literą Prawa Demeter, byłoby to do przyjęcia, ponieważ ja wtedy działałby bezpośrednio na parametrze funkcji).

Czy miałoby to jakąkolwiek różnicę, gdyby chodziło o coś nieco bardziej egzotycznego niż wywołanie toString()standardowego typu?

Odpowiadając, pamiętaj, że zmiana zachowania lub interfejsu API typu, którego servicejest zmienna, nie jest praktyczna. Ponadto, ze względu na argument, powiedzmy, że zmiana typu zwracanego getLocation()jest również niepraktyczna.

CVn
źródło

Odpowiedzi:

34

Problemem jest tutaj podpis setLocation. Jest ściśle napisany .

Aby rozwinąć: Dlaczego miałby się spodziewać String? A Stringreprezentuje dowolny rodzaj danych tekstowych . Potencjalnie może to być dowolna lokalizacja.

W rzeczywistości rodzi to pytanie: co to jest lokalizacja? Skąd mam wiedzieć, nie zaglądając do twojego kodu? Gdyby tak było URL, wiedziałbym znacznie więcej o tym, czego oczekuje ta metoda.
Może byłoby bardziej sensowne, gdyby była to klasa niestandardowa Location. Ok, na początku nie wiedziałbym, co to jest, ale w pewnym momencie (prawdopodobnie przed napisaniem this.configuration.getLocation()poświęciłbym minutę, aby dowiedzieć się, co to jest ta metoda powraca).
Oczywiście w obu przypadkach muszę szukać innego miejsca, aby zrozumieć, czego się oczekuje. Jednak w tym drugim przypadku, jeśli rozumiem, co to Locationjest, mogę użyć interfejsu API, w pierwszym przypadku, jeśli rozumiem, co to Stringjest (czego można się spodziewać), nadal nie wiem, czego oczekuje Twój interfejs API.

W mało prawdopodobnym scenariuszu, że lokalizacja jest jakimkolwiek rodzajem danych tekstowych , zinterpretowałbym to do każdego rodzaju danych, które mają reprezentację tekstową . Biorąc pod uwagę fakt, że Objectistnieje toStringmetoda, możesz z tym skorzystać, chociaż wymaga to od klientów twojego kodu znacznego skoku wiary.

Powinieneś również wziąć pod uwagę, że jest to Java, o której mówisz, która ma bardzo niewiele funkcji z założenia. Właśnie to zmusza cię do tego, by zadzwonić toStringna końcu.
Jeśli weźmiesz na przykład C #, który jest również wpisany statycznie, wtedy faktycznie możesz pominąć to wywołanie, definiując zachowanie dla niejawnej rzutowania .
W dynamicznie wpisywanych językach, takich jak Objective-C, tak naprawdę nie potrzebujesz konwersji, ponieważ dopóki wartość zachowuje się jak ciąg znaków, wszyscy są zadowoleni.

Można argumentować, że ostatnie wywołanie toStringto mniej niż połączenie, a właściwie tylko hałas generowany przez potrzebę jawności Javy. Wywołujesz metodę, którą posiada każdy obiekt Java, dlatego tak naprawdę nie kodujesz żadnej wiedzy o „odległej jednostce”, a tym samym nie naruszasz zasady najmniejszej wiedzy. Nie ma możliwości, bez względu na to getLocation, co powróci, że nie ma toStringmetody.

Ale proszę, nie używaj ciągów znaków, chyba że są one naprawdę najbardziej naturalnym wyborem (lub jeśli nie używasz języka, który nawet nie ma wyliczeń ... był tam).

back2dos
źródło
Zgadzam się z tobą, ale powiedział już, że nie może modyfikować interfejsu API usługi.
jhocking 21.09.11
1
@jhocking: Nie powiedział, że nie może. Powiedział, że to niepraktyczne. Nie zgadzam się. Próba zastosowania najlepszych praktyk w kodzie, który działa w oparciu o błędy w projektowaniu API, naprawdę ma sens tylko wtedy, gdy takie obejścia są jedyną możliwą opcją. Jednak tutaj ustawienie interfejsu API prosto jest najlepszą opcją. Usunięcie braków zawsze jest lepsze niż obejście ich.
back2dos,
no cóż, w każdym razie +1 dla „to mniej niż połączenie, a właściwie tylko hałas generowany przez żądanie
jawności
1
Dałbym to +1, nawet jeśli byłby to „ciąg wpisany”. W moim kodzie staram się przekazywać typy, które wyrażają znaczenie / intencję w jak największym stopniu, ale podczas pracy z bibliotekami i interfejsami API innych firm czasami czasami utkniesz w użyciu, co zdecydowali autorzy tego interfejsu API. I IIRC, lokalizacja może być rzeczywiście „dowolnym rodzajem danych tekstowych”, ale dla implementacji, nad którą pracuję, lokalizacja bez adresu URL jest zupełnie bez znaczenia.
CVn
1
Co do zmiany interfejsu API; jest to oprogramowanie komputerowe, więc praktycznie zawsze można zmieniać rzeczy. (Jeśli nic więcej nie zawsze możesz napisać warstwę abstrakcji.) Jednak czasami wykonanie tego wymaga zbyt dużego wysiłku, zarówno krótko-, jak i długoterminowego, aby było uzasadnione. Wówczas dokonanie takich zmian byłoby całkowicie możliwe, ale nadal niepraktyczne.
CVn
21

Prawo Demeter jest wytyczną projektową, a nie prawem, którego należy przestrzegać religijnie.

Jeśli uważasz, że twoje klasy są wystarczająco oddzielone, to nie ma nic złego w linii, this.configuration.getLocation()szczególnie jeśli, jak mówisz, zmiana innych części interfejsu API jest niepraktyczna.

Jestem prawie pewien, że klient będzie całkowicie szczęśliwy, nawet jeśli rozłożysz Prawo Demeter na kawałki, o ile tylko dostarczysz to, czego chce i na czas. Co nie jest usprawiedliwieniem tworzenia złego oprogramowania, ale przypomnieniem, aby być pragmatycznym podczas opracowywania oprogramowania.

Trasplazio Garzuglio
źródło
1
Ponieważ this.configurationjest to zmienna instancji typu znanego interfejsu, wywołanie na niej metody zdefiniowanej przez ten interfejs wydaje się w porządku, nawet przy ścisłej interpretacji. Tak, wiem, że jest to wytyczna, podobnie jak KISS, SOLID, YAGNI i tak dalej. Istnieje bardzo niewiele, jeśli w ogóle, jakichkolwiek „praw” (w sensie prawnym) w ogólnym rozwoju oprogramowania.
CVn
4
+1 za bycie pragmatycznym ;-)
Treb
1
Nie sądzę, aby prawo Dementera miało zastosowanie nawet w takim przypadku - konfiguracja to w zasadzie kontener. W każdym interfejsie API, z jakim kiedykolwiek pracowałem, zaglądanie do kontenerów jest normalnym i oczekiwanym zachowaniem.
Loren Pechtel,
2

Jedyne, co mogę wymyślić, aby nie pisać takiego kodu, to co, jeśli this.configuration.getLocation()zwróci wartość null? To zależy od otaczającego kodu i docelowych odbiorców korzystających z tego kodu. Ale, jak mówi Marco - prawo Demetera jest ogólną zasadą - dobrze jest postępować, ale nie łam się plecom, robiąc to niepotrzebnie.

Martyn
źródło
Jeśli this.configuration.getLocation()zwróci wartość null, wówczas (a) najprawdopodobniej nigdy nie zaszlibyśmy tak daleko lub (b) wydarzyło się coś katastroficznego w międzyczasie, w którym to przypadku chciałbym, aby kod zawiódł. Chociaż jest to z pewnością słuszny punkt, w tym konkretnym przypadku dość bezpiecznie jest powiedzieć, że nie ma zastosowania. Ponadto, wszystko to i wiele więcej to moduł obsługi wyjątków zaprojektowany specjalnie do radzenia sobie z tego rodzaju nieoczekiwanymi awariami.
CVn
2

Ściśle przestrzeganie prawa Demetera oznaczałoby, że powinieneś zaimplementować metodę w obiekcie konfiguracyjnym, takim jak:

function getLocationAsString() {
  return getLocation().toString();
}

ale osobiście nie zawracałbym sobie głowy, ponieważ jest to tak mała sytuacja, a poza tym twoje ręce są trochę związane z powodu interfejsu API, którego nie możesz zmienić. Reguły programowania dotyczą tego, co powinieneś zrobić, kiedy masz wybór, ale czasami nie masz wyboru.

jhocking
źródło
1
To samo sugerowane tutaj: c2.com/cgi/wiki?TrainWreck "Utwórz metodę, która reprezentuje pożądane zachowanie i mówi klientowi, co ma robić. Jest to zgodne z zasadą" mów, nie pytaj ".
heltonbiker