Pokrycie kodu podkreśla nieużywane metody - co powinienem zrobić?

Zadanie polegało na zwiększeniu zasięgu kodu istniejącego projektu Java.

Zauważyłem, że narzędzie do pokrycia kodu ( EclEmma ) wyróżniło niektóre metody, które nigdy nie są wywoływane z dowolnego miejsca.

Moja początkowa reakcja nie polega na pisaniu testów jednostkowych dla tych metod, ale na ich podkreśleniu mojemu przełożonemu / zespołowi i zapytaniu, dlaczego te funkcje są na początek.

Jakie byłoby najlepsze podejście? Napisz dla nich testy jednostkowe lub zadaj pytanie, dlaczego tam są?

1. Usunąć.
2. Popełnić.
3. Zapomnieć.

Racjonalne uzasadnienie:

1. Martwy kod jest martwy. Przez sam opis nie ma celu. W pewnym momencie mógł mieć cel, ale go nie ma, więc kod powinien zniknąć.
2. Kontrola wersji zapewnia, że ​​w (moim doświadczeniu) rzadkim wyjątkowym zdarzeniu, które ktoś przyjdzie później w poszukiwaniu tego kodu, może zostać odzyskane.
3. Jako efekt uboczny natychmiast poprawiasz zasięg kodu, nie robiąc nic (chyba że testowany jest martwy kod, co rzadko się zdarza).

Zastrzeżenie z komentarzy: Oryginalna odpowiedź zakładała, że ​​dokładnie sprawdziłeś, czy kod jest niewątpliwie martwy. IDE są omylne i istnieje kilka sposobów na wywołanie martwego kodu. Sprowadź eksperta, chyba że masz absolutną pewność.

Wszystkie pozostałe odpowiedzi oparte są na założeniu, że omawiane metody są naprawdę nieużywane. Jednak pytanie nie określało, czy ten projekt jest samowystarczalny, czy jakaś biblioteka.

Jeśli dany projekt jest biblioteką, pozornie nieużywane metody można zastosować poza projektem, a ich usunięcie spowodowałoby uszkodzenie innych projektów. Jeśli sama biblioteka jest sprzedawana klientom lub udostępniana publicznie, wyśledzenie użycia tych metod może być nawet niemożliwe.

W takim przypadku istnieją cztery możliwości:

• Jeśli metody są prywatne lub prywatne, można je bezpiecznie usunąć.
• Jeśli metody są publiczne, ich obecność może być uzasadniona, nawet bez faktycznego użycia, dla kompletności funkcji. Powinny być jednak przetestowane.
• Jeśli metody są publiczne i niepotrzebne, ich usunięcie będzie przełomową zmianą, a jeśli biblioteka będzie przestrzegać wersji semantycznej , jest to dozwolone tylko w nowej wersji głównej.
• Alternatywnie metody publiczne mogą być również przestarzałe i usunięte później. Daje to trochę czasu użytkownikom interfejsu API na przejście z przestarzałych funkcji, zanim zostaną usunięte w następnej głównej wersji.

Najpierw sprawdź, czy narzędzie do pokrywania kodu jest prawidłowe.

Miałem sytuacje, w których nie wykryły metod wywoływanych przez odwołania do interfejsu lub gdy klasa jest gdzieś dynamicznie ładowana.

Ponieważ Java jest kompilowana statycznie, usuwanie metod powinno być całkiem bezpieczne. Usuwanie martwego kodu jest zawsze dobre. Istnieje pewne prawdopodobieństwo, że istnieje jakiś szalony system refleksji, który uruchamia je w czasie wykonywania, więc najpierw sprawdź z innymi programistami, ale w przeciwnym razie usuń je.

Jakie byłoby najlepsze podejście? Napisz dla nich testy jednostkowe lub zadaj pytanie, dlaczego tam są?

Usunięcie kodu to dobra rzecz.

Jeśli nie możesz usunąć kodu, możesz z pewnością oznaczyć go jako @Deprecated , dokumentując, do którego głównego wydania chcesz usunąć metodę. Następnie możesz go usunąć „później”. W międzyczasie będzie jasne, że nie należy dodawać nowego kodu, który będzie od niego zależny.

Nie zalecałbym inwestowania w przestarzałe metody - dlatego nie ma nowych testów jednostkowych, które mogłyby tylko osiągnąć cele dotyczące zasięgu.

Różnica między nimi polega przede wszystkim na tym, czy metody są częścią opublikowanego interfejsu . Arbitralne usuwanie części opublikowanego interfejsu może być nieprzyjemną niespodzianką dla konsumentów zależnych od interfejsu.

Nie mogę rozmawiać z EclEmma, ​​ale z moich doświadczeń wynika, że ​​jedną z rzeczy, na które należy uważać, jest refleksja. Jeśli na przykład użyjesz tekstowych plików konfiguracyjnych, aby wybrać klasy / metody dostępu, użyte / nieużywane rozróżnienie może nie być oczywiste (zostałem przez to wypalony kilka razy).

Jeśli twój projekt jest liściem na wykresie zależności, wówczas argument dotyczący wycofania jest osłabiony. Jeśli twój projekt jest biblioteką, to argument za zaniechaniem jest silniejszy.

Jeśli Twoja firma korzysta z mono-repo , wówczas usuwanie jest mniejsze ryzyko niż w przypadku wielu repo.

Jak zauważono w l0b0 , jeśli metody są już dostępne w kontroli źródła, odzyskanie ich po usunięciu jest prostym ćwiczeniem. Jeśli naprawdę martwiłeś się koniecznością zrobienia tego, zastanów się, jak zorganizować swoje zatwierdzenia, aby w razie potrzeby móc odzyskać usunięte zmiany.

Jeśli niepewność jest wystarczająco wysoka, możesz rozważyć skomentowanie kodu , zamiast go usunąć. To dodatkowa praca na szczęśliwej ścieżce (gdzie usunięty kod nigdy nie jest przywracany), ale ułatwia przywracanie. Domyślam się, że powinieneś preferować proste usuwanie, dopóki nie zostaniesz przez to spalony kilka razy, co da ci wgląd w to, jak ocenić „niepewność” w tym kontekście.

pytanie, dlaczego tam są?

Czas zainwestowany w zdobywanie wiedzy niekoniecznie jest marnowany. Przeprowadzono usuwanie w dwóch krokach - najpierw poprzez dodanie i zatwierdzenie komentarza wyjaśniającego, czego dowiedzieliśmy się o kodzie, a następnie usunięcie kodu (i komentarza).

Możesz także użyć czegoś analogicznego do architektonicznych zapisów decyzji jako sposobu przechwytywania wiedzy za pomocą kodu źródłowego.

Narzędzie do pokrywania kodu nie jest wszechwiedzące, wszechwidzące. To, że twoje narzędzie twierdzi, że metoda nie jest wywoływana, nie oznacza, że nie jest wywoływana. Jest refleksja, a w zależności od języka mogą istnieć inne sposoby wywołania metody. W C lub C ++ mogą występować makra konstruujące nazwy funkcji lub metod, a narzędzie może nie widzieć wywołania. Tak więc pierwszym krokiem byłoby: Wykonaj wyszukiwanie tekstowe nazwy metody i powiązanych nazw. Zapytaj doświadczonych kolegów. Może się okazać, że jest faktycznie używany.

Jeśli nie jesteś pewien, umieść aser () na początku każdej „nieużywanej” metody. Może to się nazywa. Lub oświadczenie logowania.

Może kod jest naprawdę cenny. Może to być nowy kod, nad którym kolega pracuje od dwóch tygodni i że jutro go włączy. Nie nazywa się go dzisiaj, ponieważ wezwanie do niego zostanie dodane jutro.

Może kod jest w rzeczywistości cenny, część 2: Kod może przeprowadzać bardzo drogie testy uruchomieniowe, które mogłyby wykryć, że coś pójdzie nie tak. Kod jest włączany tylko wtedy, gdy coś pójdzie nie tak. Być może usuwasz cenne narzędzie do debugowania.

Co ciekawe, najgorsza możliwa rada „Usuń. Zaakceptuj. Zapomnij”. jest najwyżej oceniony. (Recenzje kodu? Nie robisz recenzji kodu? Co do diabła robisz programując, jeśli nie robisz recenzji kodu?)

W zależności od środowiska, w którym działa oprogramowanie, możesz się zalogować, jeśli metoda zostanie kiedykolwiek wywołana. Jeśli nie zostanie wywołany w odpowiednim czasie, metodę można bezpiecznie usunąć.

Jest to bardziej ostrożne podejście niż tylko usunięcie metody i może być przydatne, jeśli pracujesz w środowisku wrażliwym na awarie.

Logujemy się do dedykowanego #unreachable-codekanału luzu z unikalnym identyfikatorem dla każdego kandydata do usunięcia i okazało się to dość skuteczne.