[ten post jest częścią mojego minicyklu o testach, pełna lista postów: tutaj]
W komentarzach do ostatniego posta wywiązała się dyskusja na temat “a co z metodami prywatnymi?“. Odpowiedź najkrótsza z możliwych brzmi: NIC. Zainteresowanych odsyłam do tamtejszych wypowiedzi, a w niniejszej notce postaram się zawarte tam myśli rozwinąć.
Zaczynając przygodę z testami jednostkowymi często stawałem przed dylematem “jak mam przetestować funkcjonalność z metod prywatnych?“. Sporo się naszukałem i naczytałem o różnych rozwiązaniach, z czego dwa zdawały się być najpopularniejsze i najbardziej rekomendowane. I teraz, o kilka lat mądrzejszy, mogę z czystym sumieniem powiedzieć: oba były błędne.
Pierwsza rada to “skorzystaj z refleksji“. Fakt, da się w ten sposób wywołać metodę prywatną, ale czy to przypadkiem nie jest zbytnia ingerencja w sam testowany obiekt? Gdyby metoda miała z założenia być wołana z zewnątrz, to by nie była prywatna! I tu wchodzi druga rada: “upublicznij metodę“. Ale skoro dana metoda była prywatna, to jej upublicznienie tylko i wyłącznie na potrzeby testów zaśmieci nam interfejs klasy. I… śmierdzi.
Tak źle i tak niedobrze.
Moja rada, i rada Zacnych Komentatorów: nie testuj metod prywatnych. Z dwóch powodów:
1) one i tak zostaną “niejawnie” przetestowane poprzez testy metod publicznych…
2) … a jeśli nie, to oznacza że wcale nie są potrzebne, więc nie ma sensu ich testować
“Ale ja naprawdę czuję że powinienem bo są skomplikowane!”
Jeśli tak… to Twój kod wymaga zmian. Czy funkcjonalność w tej prywatnej metodzie nie jest przypadkiem zbyt przepchanym logiką helperem, który tak naprawdę w ogóle nie powinien znaleźć się w tej klasie? Pamiętaj o Single Responsibility Principle: “every object should have a single responsibility, and that responsibility should be entirely encapsulated by the class“.
Przytoczę przykład, który podałem we wspomnianych komentarzach: mamy klasę posiadającą ID, jednak nie dostaje go wprost. Zamiast tego z jakiegoś zewnętrznego systemu dajemy jej stringa i mówimy “znajdź tu sobie swoje ID“. Z życia wzięte: FIM reprezentuje referencje między obiektami poprzez taki string: “urn:uuid:b116583c-a03e-4ea8-a895-a9511a4714b3“, gdzie pierwsze dwa elementy są stałe, a trzeci to ID obiektu powiązanego.
Sytuacja wymagająca testowania metody prywatnej zakłada, że klasa taka ma zaszytą logikę poszukiwania GUIDa pod deklaracją “private string ExtractId(string source)“.
Ale czy naprawdę klasa reprezentująca jakiś obiekt w systemie powinna się takimi pierdołami zajmować? Najprawdopodobniej NIE! Wyrzućmy ten kod na zewnątrz, do, dajmy na to, FimReferenceIdExtractor. Tam nie będzie to już metodą prywatną – tam będzie to główną odpowiedzialnością klasy. I metoda “ExtractId” nie będzie prywatnym helperem, spychanym na sam dół pliku i chowanym w jakichś regionach jak bękart na królewskim dworze. Będzie po prostu częścią interfejsu! A co za tym idzie: idealnym kandydatem do przetestowania.
Do głowy przychodzą mi też momentalnie moje dawne porażki z testami kończące się stwierdzeniem:
– “moje rozwiązanie jest na tyle nietypowe że nie da się go przetestować”
– Dlaczego?
– “Bo nie umiem przetestować obsługi zdarzeń w *.aspx albo Form*.cs”
– Po pierwsze: co w tym nietypowego (niestety)? A po drugie: po co chcesz testować obsługę zdarzeń w tych plikach?
– “Bo tam mam najwięcej kodu”
– …
No i właśnie. Wtedy siedziałem całymi dniami i drapałem się po łbie kombinując jak to możliwe, że akurat MÓJ kod nie nadaje się do testowania. Dziś powiedziałbym sobie bardzo dosadnie: wyciągnij-że ten kod do klas, które naprawdę powinny go zawierać!
Z dużą dozą prawdopodobieństwa można stwierdzić, że prywatne metody wymagające osobnych testów łamią zasadę SRP i powinny być przeniesione do dedykowanego dla nich miejsca.
Bardzo fajnie ujął to w komentarzu @rek:
“Metod prywatnych nie testujemy, jeśli uważasz, że metoda prywatna powinna być otestowana to znaczy że zrobiłeś coś źle…. refactor it….. wyciągnij do osobnej klasy (SRP) wraz z testami”
A dobitniej podchwycił Paweł:
“Masz cos prywatnego, co musisz zewnetrznie przetestowac -> zabrnales w guwno (moze nawet po pas) -> refaktoryzuj ASAP“.
Może jeszcze ktoś coś doda, czy osiągnęliśmy stan globalnego współdzielenia poglądu, co w tym kraju jest sztuką niemałą?:)
Pomijając oczywiste (?) korzyści wynikające z wyciągnięcia logiki zakodowanej w prywatnej metodzie (albo raczej metodach) do zewnętrznej klasy; to jaką w tym przypadku mamy przewagę nad upublicznieniem tej prywatnej metody stricte do testów bezpośrednio? W przypadku klasy też to robimy tylko na innym poziomie. W obydwu tych przypadkach przynajmniej w .NET można użyć modyfikatora internal w połączeniu z atrybutem inernalsvisibleto i to chyba byłoby moje preferowane rozwiązanie (oczywiście jak najbardziej jestem za umieszczaniem wewnętrznej logiki gdzie faktycznie ona należy).
@daczkowski: SRP jest odpowiedzią na Twoje pytanie. Jeśli chodzi o modyfikator ‘internal’ lub atrybut ‘internalvisibleto’ to pierwsze wymusza by testy były w bibliotece z logiką (fuj!) a drugi tak samo tylko niejawnie. Pierwsze powoduje, że biblioteka rośnie i musisz całą przekazać klientowi (chyba że użyjesz ‘#if Debug’ ale to tylko zaśmiecanie kodu).
SRP jasne – tylko ja pytam, co jeśli mimo to nie chcesz, żeby akurat ta część była publicznym API dostępnym dla klientów biblioteki. Oczywiście można to udokumentować (lub właśnie nie dokumentować), ale internal (na klasach) + internalsvisibleto pozwala to zrobić explicite. I tak też mi się internalsvisibleto nie podoba, ale jest chyba mniejszym złem.
daczkowski,
Stosowałem kiedyś InternalsVisibleTo (nawet pierwszy post na tym blogu był o tym właśnie:) http://www.maciejaniserowicz.com/post/2008/02/26/Model-View-Controller-i-testy-jednostkowe.aspx )…
Jednak teraz właściwie nigdy nie widzę już takiej potrzeby. Wszelkie internale tak mi działają na nerwy w codziennym życiu (chociaż rozumiem argumenty za ich wykorzystaniem), że tego nie stosuję. Ale z drugiej strony – nie tworzę frameworków/kodu wykorzystywanego publicznie przez osoby trzecie, więc API moich dllek i tak ma scope 1 projektu.
Jak zwykle będę w mniejszości. Globalne współdzielenie poglądu? Wszystko co globalne jest złe. ;) [quote]oczywiste (?) korzyści wynikające z wyciągnięcia logiki zakodowanej w prywatnej metodzie (albo raczej metodach) do zewnętrznej klasy[/quote] Pierwszą korzyścią będzie zapewne więcej klas w aplikacji. To może być ogromna korzyść, jeśli pracodawca płaci nam od ich ilości. W przeciwnym wypadku tworzenie klas po to, by je mieć jest mocno wątpliwe. No chyba, że ktoś lubi mieć w aplikacji milion helperów, żeby gorzej mu się czytało kod i zarządzało projektem. Warto wydzielać odrębne odpowiedzialności, ale bez naciągania. Jest takie powiedzenie "nigdy nie mów nigdy". Zapewne 99% metod… Read more »
oo…… zostałem odfiltrowany :/
@rek,
Jak to odfiltrowany? W panelu komentarzy nie mam żadnej treści od Ciebie zablokowanej. Więcej: system dodał Cię do "white list"…
w takim razie gdzieś wessało komenatrz z linkiem do rozmowy Uncle Boba na temat testowania metod prywatnych – bywa. Odszukam i wyślę jeszcze raz
Opinia Roberta C. Martina na temat testowania metod prywatnych http://vimeo.com/20388419 (od 39 minuty)
procent: no i sprawa się wyjaśniła, zaginiony komentarz o którym pisałem wcześniej jest tutaj http://www.maciejaniserowicz.com/post/2011/12/08/UT-5-Kiedy-testowac.aspx najwyraźniej późno już było i się mi okna pomyliły. Mea culpa
@somekind: budując zaproponowaną przez Ciebie klasę, już na początku wydzieliłbym mechanizmy odpowiedzialne za rozbijanie macierzy na trzy wymagane przez metodę Jacobiego (niewiele pamiętam już z algebry, opis metody znalazłem w sieci). Umieściłbym je jako metody statyczne powiedzmy w klasie … Transformations przestrzeni nazw Matrix ;). Byłyby one oczywiście publiczne (a może byłaby tylko jedna metoda, ale zwracająca kolekcję trzech macierzy?). I to dla klasy Transformations powstałby pierwszy test. Dopiero jego zaliczenie upoważniałoby mnie do napisania klasy właściwej z jedną metodą [i]Calculate[/i]. Jak widać nie ma ani przerażającego przyrostu klas, ani konieczności testowania metod prywatnych, bo tak naprawdę nie są one… Read more »
Jeszcze korekta dla autora wpisu. O ile dobrze zrozumiałem, to w trzecim akapicie zamiast [i]to by nie była publiczna![/i], powinno być: [i]to by nie była [u]prywatna[/u]![/i]
PaSkol,
Thx, poprawione
@PaSkol, Nazwanie przestrzeni nazw Matrix w programie numerycznym, to byłby chyba strzał w stopę, bo znacznie utrudniłoby korzystanie z klasy Matrix. ;) Co do zasadniczej części postu – jeśli chodzi Ci o macierze wyznaczane w każdej iteracji (wg mojej wiedzy oznaczane przez A i W), to one są po prostu wynikiem prostych mnożeń, i nie potrzebują oddzielnych metod pomocniczych. Te są potrzebne, jak już pisałem, do przeszukiwania macierzy w poszukiwaniu największego elementu i zwrócenia jego indeksów. Ok, można zrobić klasę MatrixSearcher z metodami FindElementByCośtam1 i 2, ale jaki to ma sens, jeżeli metody te (a zarazem i cała klasa) byłyby… Read more »
somekind,
Nie mogę się zgodzić z tym rozumowaniem. Duża liczba klas nie ma negatywnego przełożenia na organizacją kodu. Gdyby tak było to każdy program posiadałby jedną klasę i wszyscy byliby super-zorganizowani. Klasa nie jest też worem na "reużywalny" kod i nowej klasy nie tworzy się tylko po to, aby wykorzystać jej funkcjonalność w kilku miejscach.
Pytanie: jak przetestować metodę X w projekcie, który jest biblioteką.
Sytuacja: mam DLL z publicznym interfejsem do WebService’u: GetItem(), SaveItems(Item[] items), gdzie Item jest skomplikowaną klasą typu DTO. Jedno z pól tej klasy to SN.
Przed wysłaniem do WS pole SN musi być walidowane gość skomplikowanym algorytmem, który jest w osobnej klasie typu internal SNHelper. Klasa ta nie może być publiczna!
X = SNHelper.Validate()
Nadmienię, że testowanie przez wywołanie SaveItems() jest zbyt skomplikowane, ponieważ walidacja SN to tylko czubek góry lodowej.
Możliwe rozwiązania – bez faworyzowania:
a) kompilacja warunkowa dla testów, która upublicznia SNHelper;
b) odwołanie przez reflection;
c) ?