W ten sposób piszę ten kod, który można przetestować, ale czy coś jest z nim nie tak?

13

Mam interfejs o nazwie IContext. W tym celu tak naprawdę nie ma znaczenia, co robi, oprócz następujących czynności:

T GetService<T>();

Metoda ta polega na sprawdzeniu bieżącego kontenera DI aplikacji i próbie rozwiązania zależności. Myślę, że dość standardowy.

W mojej aplikacji ASP.NET MVC mój konstruktor wygląda tak.

protected MyControllerBase(IContext ctx)
{
    TheContext = ctx;
    SomeService = ctx.GetService<ISomeService>();
    AnotherService = ctx.GetService<IAnotherService>();
}

Więc zamiast dodawania wielu parametrów konstruktora dla każdej usługi (bo to będzie naprawdę denerwujące i czasochłonne dla programistów Rozszerzenie stosowania) Używam tej metody, aby uzyskać usług.

Teraz jest źle . Jednak w tej chwili usprawiedliwiam to w mojej głowie - mogę to wyśmiewać .

Mogę. IContextTestowanie kontrolera nie będzie trudne . W każdym razie musiałbym:

public class MyMockContext : IContext
{
    public T GetService<T>()
    {
        if (typeof(T) == typeof(ISomeService))
        {
            // return another mock, or concrete etc etc
        }

        // etc etc
    }
}

Ale jak powiedziałem, wydaje się to złe. Wszelkie myśli / nadużycia są mile widziane.

LiverpoolsNumber9
źródło
8
Nazywa się to Lokalizatorem usług i nie podoba mi się to. Na ten temat napisano dużo - na początek patrz martinfowler.com/articles/injection.html i blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern .
Benjamin Hodgson
Z artykułu Martina Fowlera: „Często słyszałem skargę, że tego rodzaju lokalizatory usług są złą rzeczą, ponieważ nie można ich przetestować, ponieważ nie można zastąpić ich implementacjami. Z pewnością można je źle zaprojektować, aby się w to zaangażować rodzaj kłopotów, ale nie musisz. W tym przypadku instancja lokalizatora usług jest po prostu zwykłym posiadaczem danych. Mogę łatwo utworzyć lokalizator z testowymi implementacjami moich usług. ” Czy możesz wyjaśnić, dlaczego ci się nie podoba? Może w odpowiedzi?
LiverpoolsNumber9
8
Ma rację, to zły projekt. To proste: public SomeClass(Context c). Ten kod jest dość jasny, prawda? To stwierdza, that SomeClasszależy od Context. Err, ale czekaj, to nie! Zależy tylko od zależności, Xktórą uzyskuje z kontekstu. Oznacza to, że za każdym razem, gdy wprowadzasz zmiany Context, może się zepsuć SomeObject, nawet jeśli zmieniłeś tylko Contexts Y. Ale tak, wiesz, że tylko zmienione Ynie X, więc SomeClassjest w porządku. Ale pisanie dobrego kodu nie polega na tym , co wiesz, ale na tym, co nowy pracownik wie, kiedy patrzy na twój kod po raz pierwszy.
valenterry
@DocBrown Dla mnie to właśnie powiedziałem - nie widzę tutaj różnicy. Czy możesz wyjaśnić dalej?
valenterry
1
@DocBrown Widzę teraz twój punkt widzenia. Tak, jeśli jego Kontekst jest tylko pakietem wszystkich zależności, to nie jest to zły projekt. Może to być jednak złe nazewnictwo, ale to także tylko założenie. OP powinien wyjaśnić, czy istnieje więcej metod (wewnętrznych obiektów) kontekstu. Dyskusja na temat kodu jest również w porządku, ale to jest programiści.stackexchange, więc dla mnie powinniśmy również spróbować zobaczyć „za” rzeczami, aby ulepszyć OP.
valenterry

Odpowiedzi:

5

Posiadanie jednego zamiast wielu parametrów w konstruktorze nie jest problematyczną częścią tego projektu . Tak długo, jak twoja IContextklasa jest niczym innym, jak tylko fasada usługi , szczególnie w celu zapewnienia zależności używanych w MyControllerBasecałym kodzie, a nie ogólny lokalizator usług używany w całym kodzie, ta część twojego kodu jest w porządku.

Twój pierwszy przykład może zostać zmieniony na

protected MyControllerBase(IContext ctx)
{
    TheContext = ctx;
    SomeService = ctx.GetSomeService();
    AnotherService = ctx.GetAnotherService();
}

nie byłaby to znacząca zmiana projektu MyControllerBase. To, czy ten projekt jest dobry czy zły, zależy tylko od tego, czy chcesz

  • upewnij się TheContext, SomeServicei AnotherServicezawsze wszystko zainicjowany z makiety obiektów, lub wszystkie z nich z rzeczywistych obiektów
  • lub, aby umożliwić ich inicjalizację przy użyciu różnych kombinacji 3 obiektów (co oznacza, że ​​w tym przypadku musisz przekazać parametry indywidualnie)

Zatem użycie tylko jednego parametru zamiast 3 w konstruktorze może być w pełni uzasadnione.

Problemem jest publiczne IContextujawnienie GetServicemetody. IMHO powinieneś tego unikać, zamiast tego trzymaj się wyraźnie „metod fabrycznych”. Więc czy będzie w porządku zaimplementować metody GetSomeServicei GetAnotherServicez mojego przykładu za pomocą lokalizatora usług? IMHO to zależy. Tak długo, jak długo IContextklasa będzie po prostu tworzyła prostą abstrakcyjną fabrykę, której celem jest dostarczenie jawnej listy obiektów usług, co jest IMHO dopuszczalne. Fabryki abstrakcyjne to zazwyczaj tylko „klej” kod, który sam nie musi być testowany jednostkowo. Niemniej jednak powinieneś zadać sobie pytanie, czy w kontekście metod takich jak GetSomeService, czy naprawdę potrzebujesz lokalizatora usług, czy też jawne wywołanie konstruktora nie byłoby prostsze.

Uważajcie więc, kiedy trzymacie się projektu, w którym IContextimplementacja jest po prostu owinięciem publicznej, ogólnej GetServicemetody, pozwalającej rozwiązać dowolne zależności przez arbitralne klasy, wtedy wszystko stosuje się tak, jak napisał @BenjaminHodgson w swojej odpowiedzi.

Doktor Brown
źródło
Zgadzam się z tym. Problemem w tym przykładzie jest GetServicemetoda ogólna . Lepsze jest refaktoryzowanie do metod jawnie nazwanych i pisanych na maszynie. Jeszcze lepiej byłoby IContextwyraźnie określić zależności implementacji w konstruktorze.
Benjamin Hodgson,
1
@BenjaminHodgson: czasami może być lepiej, ale nie zawsze jest lepiej. Zauważasz zapach kodu, gdy lista parametrów ctor staje się coraz dłuższa. Zobacz moją poprzednią odpowiedź tutaj: programmers.stackexchange.com/questions/190120/…
Doc Brown
@DocBrown „zapachowy kod” nadmiernego wtrysku konstruktora wskazuje na naruszenie SRP, co jest prawdziwym problemem. Po prostu zestawienie kilku usług w klasę Fasada, tylko po to, aby ujawnić je jako właściwości, nie robi nic, aby rozwiązać źródło problemu. Zatem Fasada nie powinno być proste otoki wokół innych elementów, ale powinien idealnie oferują uproszczonego interfejsu API, które je spożywają (lub powinno uprościć je w jakiś inny sposób) ...
AlexFoxGill
... Pamiętaj, że „zapach kodu”, taki jak nadmierne wstrzyknięcie konstruktora, nie stanowi problemu sam w sobie. To tylko wskazówka, że ​​w kodzie jest głębszy problem, który zwykle można rozwiązać przez rozsądne
przefakturowanie po
Gdzie byłeś, kiedy Microsoft to upiekł IValidatableObject?
RubberDuck 12.04.16
15

Ten projekt jest znany jako Service Locator * i nie podoba mi się. Istnieje wiele argumentów przeciwko niemu:

Lokalizator usług łączy Cię z Twoim kontenerem . Używając regularnego wstrzykiwania zależności (gdy konstruktor wyraźnie określa zależności), możesz od razu zastąpić swój kontener innym lub powrócić do newwyrażeń. Z twoim IContexttak naprawdę nie jest to możliwe.

Lokalizator usług ukrywa zależności . Jako klient bardzo trudno jest powiedzieć, czego potrzebujesz, aby zbudować instancję klasy. Potrzebujesz jakiegoś rodzaju IContext, ale musisz także ustawić kontekst, aby zwrócić poprawne obiekty w celu wykonania MyControllerBasepracy. Nie jest to wcale oczywiste z podpisu konstruktora. Dzięki zwykłemu DI kompilator mówi dokładnie to, czego potrzebujesz. Jeśli twoja klasa ma wiele zależności, powinieneś poczuć ten ból, ponieważ skłoni cię to do refaktoryzacji. Lokalizator usług ukrywa problemy ze złymi projektami.

Lokalizator usług powoduje błędy w czasie wykonywania . Jeśli zadzwonisz GetServicez parametrem złego typu, otrzymasz wyjątek. Innymi słowy, twoja GetServicefunkcja nie jest funkcją całkowitą. (Funkcje całkowite są pomysłem ze świata FP, ale w zasadzie oznacza to, że funkcje zawsze powinny zwracać wartość.) Lepiej pozwolić kompilatorowi pomóc i powiedzieć, kiedy pomyliłeś zależności.

Lokalizator usług narusza zasadę substytucji Liskowa . Ponieważ jego zachowanie różni się w zależności od argumentu typu, Service Locator może być postrzegany tak, jakby efektywnie dysponował nieskończoną liczbą metod w interfejsie! Ten argument został szczegółowo opisany tutaj .

Lokalizator usług jest trudny do przetestowania . Podałeś przykład podróbki IContextdo testów, co jest w porządku, ale na pewno lepiej nie pisać tego kodu w pierwszej kolejności. Po prostu wstrzyknij swoje fałszywe zależności bezpośrednio, bez przechodzenia przez lokalizator usług.

Krótko mówiąc, po prostu nie rób tego . Wydaje się, że to uwodzicielskie rozwiązanie problemu klas z wieloma zależnościami, ale na dłuższą metę po prostu sprawisz, że twoje życie będzie nieszczęśliwe.

* Definiuję Service Locator jako obiekt za pomocą ogólnej Resolve<T>metody, która jest w stanie rozwiązać dowolne zależności i jest używana w całej bazie kodu (nie tylko w katalogu głównym). Nie jest to to samo, co Service Facade (obiekt, który łączy jakiś niewielki znany zestaw zależności) lub Abstract Factory (obiekt, który tworzy instancje jednego typu - typ Abstract Factory może być ogólny, ale metoda nie jest) .

Benjamin Hodgson
źródło
1
Planujesz korzystać z wzorca Service Locator (na co się zgadzam). Ale tak naprawdę, w przykładzie PO, MyControllerBasenie jest on ani sprzężony z konkretnym kontenerem DI, ani też nie jest to „naprawdę” przykład anty-wzorca Service Locator.
Doc Brown
@DocBrown Zgadzam się. Nie dlatego, że to ułatwia mi życie, ale dlatego, że większość przykładów podanych powyżej nie dotyczy mojego kodu.
LiverpoolsNumber9
2
Dla mnie znakiem rozpoznawczym anty-wzorca Service Locator jest GetService<T>metoda ogólna . Rozwiązywanie arbitralnych zależności to prawdziwy zapach, który jest obecny i poprawny w przykładzie PO.
Benjamin Hodgson,
1
Innym problemem związanym z używaniem Lokalizatora usług jest to, że zmniejsza elastyczność: możesz mieć tylko jedną implementację każdego interfejsu usługi. Jeśli zbudujesz dwie klasy, które opierają się na IFrobnicator, ale później zdecydujesz, że należy użyć oryginalnej implementacji DefaultFrobnicator, ale druga naprawdę powinna używać dekoratora CacheingFrobnicator wokół niego, musisz zmienić istniejący kod, a jeśli jesteś wstrzykiwanie zależności bezpośrednio, wszystko co musisz zrobić, to zmienić kod instalacyjny (lub plik konfiguracyjny, jeśli używasz środowiska DI). Jest to zatem naruszenie OCP.
Jules
1
@DocBrown Ta GetService<T>()metoda zezwala na żądanie dowolnych klas: „Metoda ta polega na spojrzeniu na bieżący kontener DI aplikacji i próbie rozwiązania zależności. Myślę, że dość standardowa”. . Odpowiedziałem na twój komentarz na początku tej odpowiedzi. Jest to w 100% Lokalizator usług
AlexFoxGill
5

Najlepsze argumenty przeciwko anty-wzorcowi Service Locator są wyraźnie stwierdzone przez Marka Seemanna, więc nie będę się zbytnio zastanawiał, dlaczego to zły pomysł - jest to podróż edukacyjna, którą musisz poświęcić trochę czasu na zrozumienie siebie (I również polecam książkę Marka ).

OK, aby odpowiedzieć na pytanie - ponownie określmy swój rzeczywisty problem :

Zamiast dodawać wiele konstruktorów dla każdej usługi (ponieważ będzie to naprawdę denerwujące i czasochłonne dla programistów rozszerzających aplikację) używam tej metody, aby uzyskać usługi.

Istnieje pytanie, które rozwiązuje ten problem na StackOverflow . Jak wspomniano w jednym z komentarzy:

Najlepsza uwaga: „Jedną ze wspaniałych korzyści płynących z iniekcji konstruktora jest to, że łamanie zasady pojedynczej odpowiedzialności jest rażąco oczywiste”.

Szukasz niewłaściwego miejsca na rozwiązanie swojego problemu. Ważne jest, aby wiedzieć, kiedy klasa robi zbyt wiele. W twoim przypadku mocno podejrzewam, że nie ma potrzeby stosowania „kontrolera podstawowego”. W rzeczywistości w OOP prawie zawsze nie ma potrzeby dziedziczenia . Różnice w zachowaniu i współdzielonej funkcjonalności można osiągnąć całkowicie poprzez odpowiednie użycie interfejsów, co zwykle skutkuje lepiej faktoryzowanym i enkapsulowanym kodem - i nie ma potrzeby przekazywania zależności konstruktorom nadklasy.

We wszystkich projektach, nad którymi pracowałem, w których istnieje Kontroler bazowy, zostało to zrobione wyłącznie w celu udostępnienia dogodnych właściwości i metod, takich jak IsUserLoggedIn()i GetCurrentUserId(). STOP . To jest okropne niewłaściwe wykorzystanie dziedzictwa. Zamiast tego utwórz komponent, który udostępnia te metody i weź zależność od niego tam, gdzie jest potrzebny. W ten sposób komponenty pozostaną testowalne, a ich zależności będą oczywiste.

Oprócz czegokolwiek innego, używając wzorca MVC, zawsze zalecałbym chudych kontrolerów . Możesz przeczytać więcej na ten temat tutaj, ale istota wzorca jest prosta, że ​​kontrolery w MVC powinny robić tylko jedno: obsłużyć argumenty przekazane przez środowisko MVC, delegując inne obawy na inny komponent. To znowu jest zasada pojedynczej odpowiedzialności w pracy.

Naprawdę pomocna byłaby znajomość twojego przypadku użycia, aby dokonać dokładniejszej oceny, ale szczerze mówiąc, nie mogę wymyślić żadnego scenariusza, w którym klasa podstawowa byłaby lepsza niż dobrze uwzględnione zależności.

AlexFoxGill
źródło
+1 - to nowe spojrzenie na pytanie, na które żadna z pozostałych odpowiedzi tak naprawdę nie ujęła odpowiedzi
Benjamin Hodgson
1
„Jedną ze wspaniałych zalet Constructor Injection jest to, że łamanie zasady pojedynczej odpowiedzialności jest rażąco oczywiste” Uwielbiam to. Naprawdę dobra odpowiedź. Nie zgadzaj się z tym wszystkim. Mój przypadek użycia jest, co zabawne, nie powielaniem kodu w systemie, który będzie miał ponad 100 kontrolerów (przynajmniej). Jednak w odniesieniu do SRP - każda wstrzykiwana usługa ma jedną odpowiedzialność, podobnie jak kontroler, który korzysta z nich wszystkich - w jaki sposób jeden refraktor?
LiverpoolsNumber9
1
@ LiverpoolsNumber9 Kluczem jest przeniesienie funkcjonalności z BaseController do zależności, dopóki nie pozostanie nic w BaseController oprócz protecteddelegacji jednowierszowych do nowych komponentów. Następnie można stracić BaseController i zastąpienia tych protectedmetod z bezpośrednich połączeń z zależnościami - to uprości swój model i dokonać wszystkich zależności wyraźne (co jest bardzo dobrą rzeczą w projekcie z 100s kontrolerów!)
AlexFoxGill
@ LiverpoolsNumber9 - jeśli możesz skopiować część BaseController do pastebin, mógłbym podać konkretne sugestie
AlexFoxGill
-2

Dodaję do tego odpowiedź na podstawie wkładu wszystkich innych. Wielkie dzięki wszystkim. Najpierw moja odpowiedź: „Nie, nie ma w tym nic złego”.

Odpowiedź doktora Browna na „Service Facade” Przyjąłem tę odpowiedź, ponieważ to, czego szukałem (jeśli odpowiedź brzmiała „nie”), to kilka przykładów lub rozwinięcie tego, co robiłem. Podał to, sugerując, że A) ma nazwę, a B) są prawdopodobnie lepsze sposoby na zrobienie tego.

Odpowiedź Benjamina Hodgsona na „Lokalizator usług” Mimo, że doceniam wiedzę, którą tutaj zdobyłem, to nie jestem „lokalizatorem usług”. Jest to „fasada usług”. Wszystko w tej odpowiedzi jest poprawne, ale nie w moich okolicznościach.

Odpowiedzi USR

Zajmę się tym bardziej szczegółowo:

W ten sposób rezygnujesz z wielu statycznych informacji. Odradzasz podejmowanie decyzji dotyczących środowiska wykonawczego, tak jak robi to wiele języków dynamicznych. W ten sposób tracisz weryfikację statyczną (bezpieczeństwo), obsługę dokumentacji i narzędzi (autouzupełnianie, refaktoryzacje, znajdowanie zastosowań, przepływ danych).

Nie tracę żadnego oprzyrządowania i nie tracę żadnego „statycznego” pisania. Fasada usługi zwróci to, co skonfigurowałem w moim kontenerze DI lub default(T). A to, co zwraca, jest „wpisywane”. Odbicie jest zamknięte.

Nie rozumiem, dlaczego dodanie dodatkowych usług jako argumentów konstruktora jest dużym obciążeniem.

Z pewnością nie jest to „rzadkie”. Ponieważ używam kontrolera podstawowego , za każdym razem, gdy muszę zmienić jego konstruktor, być może będę musiał zmienić 10, 100, 1000 innych kontrolerów.

Jeśli użyjesz struktury wstrzykiwania zależności, nie będziesz musiał nawet ręcznie przekazywać wartości parametrów. Z drugiej strony tracisz statyczne zalety, ale nie tak wiele.

Używam zastrzyku zależności. O to chodzi.

I na koniec, komentarz Julesa do odpowiedzi Benjamina nie tracę żadnej elastyczności. To moja fasada usług. Mogę dodać tyle parametrów, GetService<T>ile chcę, aby rozróżnić różne implementacje, tak jak zrobiłby to podczas konfigurowania kontenera DI. Na przykład mógłbym zmienić, GetService<T>()aby GetService<T>(string extraInfo = null)obejść ten „potencjalny problem”.


W każdym razie jeszcze raz dziękuję wszystkim. To było naprawdę przydatne. Twoje zdrowie.

LiverpoolsNumber9
źródło
4
Nie zgadzam się, że masz tutaj fasadę usługi. GetService<T>()Metoda jest w stanie (próba) rozstrzyga arbitralnych zależnościami . To czyni go lokalizatorem usług, a nie fasadą usług, jak wyjaśniłem w przypisie mojej odpowiedzi. Jeśli miałbyś zastąpić go małym zestawem GetServiceX()/ GetServiceY()metod, jak sugeruje @DocBrown, byłby to Fasada.
Benjamin Hodgson
2
Powinieneś przeczytać i zwrócić szczególną uwagę na ostatnią część tego artykułu na temat Lokalizatora usług abstrakcyjnych , który jest zasadniczo tym, co robisz. Widziałem, jak ten anty-wzór niszczy cały projekt - zwróć szczególną uwagę na cytat „pogorszy to twoje życie jako programisty konserwacji, ponieważ będziesz musiał użyć znacznej ilości mocy mózgu, aby zrozumieć konsekwencje każdej wprowadzonej zmiany „
AlexFoxGill
3
W przypadku pisania statycznego - brakuje Ci sensu. Jeśli spróbuję napisać kod, który nie zapewnia klasy używającej DI z wymaganym parametrem konstruktora, nie skompiluje się. To statyczne zabezpieczenie w czasie kompilacji przed problemami. Jeśli napiszesz swój kod, dostarczając konstruktorowi tekst IContext, który nie został poprawnie skonfigurowany do dostarczenia argumentu, którego faktycznie potrzebuje, wówczas skompiluje się i zakończy się niepowodzeniem w czasie wykonywania
Ben Aaronson
3
@ LiverpoolsNumber9 Cóż więc, dlaczego w ogóle używa się silnie pisanego języka? Błędy kompilatora to pierwsza linia obrony przed błędami. Testy jednostkowe są drugim. Twoja nie zostałaby wykryta przez testy jednostkowe, ponieważ jest to interakcja między klasą i jej zależnością, więc byłbyś w trzeciej linii obrony: testach integracyjnych. Nie wiem, jak często je uruchamiasz, ale teraz mówisz o opinii na temat rzędu minut lub więcej, a nie milisekundach, jakie zajmuje IDE podkreślenie błędu kompilatora.
Ben Aaronson
2
@ LiverpoolsNumber9 źle: twoje testy muszą teraz wypełnić IContexti wstrzyknąć to, a co najważniejsze: jeśli dodasz nową zależność, kod będzie nadal się kompilował - nawet jeśli testy zakończą się niepowodzeniem w czasie wykonywania
AlexFoxGill