Jak bezpiecznie usunąć fragment kodu, który wygląda na to, że nigdy nie został wprowadzony?

125

Znalazłeś trochę kodu, który wygląda na zbyteczny, a kompilator tego nie zauważa. Co robisz, aby mieć pewność (lub jak najbardziej pewne), że usunięcie tego kodu nie spowoduje regresji.

Przychodzą mi na myśl dwa pomysły.

  1. „Po prostu” użyj dedukcji na podstawie tego, czy kod wygląda na to, że powinien zostać wykonany. Czasami jednak może to być złożona, czasochłonna praca, nieco ryzykowna (twoja ocena jest podatna na błędy), ponieważ nie przynosi istotnych zysków biznesowych.

  2. Umieść logowanie w tej sekcji kodu i zobacz, jak często jest wprowadzane w praktyce. Po wystarczającej liczbie wykonań powinieneś mieć pewność, że usunięcie kodu jest bezpieczne.

Czy są jakieś lepsze pomysły lub coś w rodzaju podejścia kanonicznego?

Brad Thomas
źródło
55
Przydatne może być przejrzenie historii kontroli wersji: Kiedy utworzono „martwy kod”? Czy wyglądał martwy, kiedy został stworzony? Czy inny kod (który został zmodyfikowany lub usunięty) został sprawdzony, w którym go używał, w tym samym czasie, w którym został utworzony?
ChrisW
9
Kompilatorowi nie można ufać, jeśli w grę wchodzi jakakolwiek forma metaprogramowania, taka jak refleksja. Prawidłowe grep katalogu podstawowego jest niezbędną praktyką. Często zdarza się, że wiele aplikacji współużytkuje kod, więc pamiętaj o grepowaniu na właściwej warstwie katalogów.
Adam Caviness
16
Zapytałbym: „Po co zawracać sobie głowę usunięciem?”. Jeśli jest używany w jakimś przypadku krawędzi, który jest nazywany raz na niebieskim księżycu, zepsułeś rzeczy. Jeśli nigdy nie zostanie użyty i pozostawisz go tam, szkoda jest taka, że ​​plik wykonywalny jest nieco większy. Jeśli nie dotyczy to niektórych urządzeń osadzonych, to jest wielka sprawa? Przyklej komentarz i przejdź do produktywnego wykorzystania czasu.
mickeyf
40
@mickeyf Szkoda polega na tym, że ktoś musi zachować ten kod. Oznacza to, że programista musi zrozumieć, co robi. Deweloper musi go zmodyfikować, jeśli coś się zmieni, a otaczający go kod musi zrobić coś innego. Uwierz mi, kiedy zostawiasz leżące wokół cruft, jest to poważny ból głowy dla każdego, kto próbuje później coś wymyślić.
jpmc26
9
@MichaelJ. Chodzi mi o to, że przede wszystkim nie powinno tam być. A jeśli go opuszczę, sytuacja będzie gorsza: teraz jakiś inny programista z dalszej linii również musi to zbadać. Im dłużej pozostaje, tym więcej czasu poświęca się na jego zrozumienie. Jest to koszt zbędnego kodu.
jpmc26

Odpowiedzi:

112

W moim idealnym świecie fantasy, w którym mam 100% pokrycia testem jednostkowym, po prostu go usunę, uruchomię testy jednostkowe, a gdy żaden test nie zmieni koloru na czerwony, to go popełniam.

Ale niestety muszę budzić się każdego ranka i stawić czoła trudnej rzeczywistości, w której wiele kodu albo nie ma testów jednostkowych, albo kiedy już tam są, nie można ufać, że naprawdę obejmie każdy możliwy przypadek krawędzi. Rozważę więc ryzyko / nagrodę i dojdę do wniosku, że po prostu nie warto:

  • Nagroda: kod jest nieco łatwiejszy do utrzymania w przyszłości.
  • Ryzyko: Złam kod w jakimś niejasnym przypadku, o którym nie myślałem, spowoduj incydent, gdy najmniej się go spodziewam, i popełniam błąd, ponieważ musiałem zadowolić się jakością kodu OCD i dokonać zmiany bez zauważalnej wartości biznesowej do wszystkich interesariuszy, którzy sami nie są programistami.
Philipp
źródło
249
Obserwując stan dużych projektów programowych, w których inżynierowie przez dziesięć do dwunastu lat przeważnie stosowali podejście „nie warte swojej ceny” do usuwania prawdopodobnie zbędnego kodu, nie mogę zalecić takiego podejścia.
njuffa
125
Ponadto testy jednostkowe nie mówią nic o tym, czy ten kod jest faktycznie wykorzystywany w produkcji. Kod może być doskonale przetestowany, ale nadal nie jest używany w produkcji, dlatego jest zbyteczny.
knub
8
Z mojego doświadczenia wynika, że ​​stan takich projektów nigdy nie wynika z podejścia „nie warto” naprawiania kodu po fakcie, zawsze jest to spowodowane przede wszystkim pisaniem złego kodu. Błędem jest pisanie złego kodu, ale również błędem (bardziej zaawansowanym błędem) jest myślenie, że czyszczenie kodu zawsze jest tego warte.
Weyland Yutani,
42
@Weyland Yutani To wcale nie odpowiada mojemu doświadczeniu. Większość prawdopodobnego zbędnego kodu, który widziałem, była wystarczająco dobrze napisana i całkowicie uzasadniona, biorąc pod uwagę specyfikacje oprogramowania, platformy sprzętowe lub wersje systemu operacyjnego istniejące dziesięć lat wcześniej, potrzebne do obejścia błędów lub ograniczeń w łańcuchu narzędzi lub sprzętu dostępnego wcześniej itp. ,
njuffa,
20
Odpowiedź nie uwzględnia jednej z najważniejszych i najważniejszych korzyści płynących z dokonywania zmian i wprowadzania zmian: uczenia się. Po zapoznaniu się z nią możesz dodać strukturę (komentarze, testy), aby pomóc programistom w późniejszym czasie. Wybór, aby czegoś nie zmieniać, ponieważ nie wiesz, jakie są konsekwencje, to programowanie kultu ładunku, nawet jeśli wybór ten nie polega na usunięciu czegoś.
kojiro
84

Proces ten składa się z dwóch połówek. Pierwszy potwierdza, że ​​kod jest rzeczywiście martwy. Drugim jest zrozumienie kosztów popełnienia błędu i upewnienie się, że są one odpowiednio złagodzone.

Wiele odpowiedzi tutaj ma świetne rozwiązania dla pierwszej połowy. Narzędzia takie jak analizatory statyczne doskonale nadają się do identyfikowania martwego kodu. grepw niektórych przypadkach może być twoim przyjacielem. Jednym z nietypowych kroków, które często podejmuję, jest próba ustalenia pierwotnego celu kodu. O wiele łatwiej jest argumentować, że „X nie jest już cechą naszego produktu, a segment kodu Y został zaprojektowany do obsługi funkcji X”, niż mówiąc „Nie widzę żadnego celu dla segmentu kodu Y”.

Druga połowa jest kluczowym krokiem do złamania jakiegokolwiek impasu w kwestii tego, czy powinieneś usunąć kod. Musisz zrozumieć, jakie są konsekwencje błędnej odpowiedzi. Jeśli ludzie umrą, jeśli źle odpowiesz, zwróć uwagę! Może to dobry moment, aby zaakceptować rozwój cruft kodu i starać się nie pisać więcej cruft. Jeśli ludzie nie umrą, zadaj sobie pytanie, jak wybaczają Twoi użytkownicy. Czy możesz wysłać im poprawkę, jeśli coś zepsułeś i utrzymałeś relacje z klientami? Czy masz zespół Q&A, który otrzymuje wynagrodzenie za znalezienie takich problemów? Tego rodzaju pytania są niezbędne, aby zrozumieć, jak pewna musisz być, zanim naciśniesz klawisz usuwania.

W komentarzach rmun wskazał na doskonałe sformułowanie pojęcia zrozumienia pierwotnego celu kodu przed jego usunięciem. Ten cytat jest teraz znany jako Ogrodzenie Chestertona . Chociaż jest on zbyt duży, aby można go było cytować bezpośrednio w komentarzu, myślę, że zasługuje na to, aby go tutaj odpowiednio podać:

W kwestii reformowania rzeczy, w odróżnieniu od ich deformowania, istnieje jedna prosta i prosta zasada; zasada, która prawdopodobnie będzie nazywana paradoksem. W takim przypadku istnieje pewna instytucja lub prawo; powiedzmy, dla uproszczenia, ogrodzenie lub bramę wzniesioną po drugiej stronie drogi. Nowocześniejszy typ reformatora podchodzi do tego wesoło i mówi: „Nie widzę zastosowania tego; usuńmy go. ”Na co bardziej inteligentny typ reformatora zrobi dobrze, aby odpowiedzieć:„ Jeśli nie widzisz jego użycia, na pewno nie pozwolę ci go usunąć. Odejdź i pomyśl. Potem, kiedy będziesz mógł wrócić i powiedzieć mi, że widzisz jego użycie, pozwolę ci go zniszczyć.

Cort Ammon
źródło
14
Ogrodzenie Chestertona zakłada, że ​​odkrycie następuje wyłącznie poprzez myślenie i nie oferuje ucieczki - filozofia starożytnej Grecji. Metoda naukowa jest bardziej nowoczesna - opracuj kontrolowany eksperyment, aby określić konsekwencje usunięcia konstrukcji. Z pewnością nie zaproponowałbym nonszalancko, aby ten eksperyment odbył się w produkcji. Ale jeśli nie ma alternatywy, a koszt prób w produkcji jest bardzo wysoki, cóż, wolałbym szybko opuścić miejsce pracy, które nie zapewniło mi żadnego bezpiecznego sposobu dowiedzenia się, dlaczego istnieje jakiś kod - ryzyko osobiste przy takiej pracy jest zbyt wysokie .
kojiro
15
@kojiro To bardzo prawda. Lubię myśleć, że Chesterton byłby w porządku, gdybym jako „nowoczesny reformator” przyszedł i powiedział: „Nie wiem, dlaczego ten płot jest tutaj, przyznaję. Ale chciałbym go pomalować na czerwono, aby zobaczyć, czy która dostarcza mi wszelkich nowych informacji, „Chesterton prawdopodobnie byłby z tego zadowolony.
Cort Ammon
41
Metoda dekodowania zegara trybu jądra systemu VMS SYS $ ASCTIM zawiera wiersz kodu, który pomaga skorygować gwiezdny dryf równonocy wiosennej. Żaden z testów jednostkowych DEC nie wykonuje tego kodu. Uruchomił się raz - poprawnie - w dniu 2000-02-28 o 23: 59: 59: 999. Nie uruchomi się ponownie do roku 2400. Nie usuwaj go.
AI Breveleri
13
@AIBreveleri Mam nadzieję, że w samym kodzie znajduje się komentarz wyjaśniający to, a nie tylko tutaj na StackExchange: D
Kyle Strand
11
@KyleStrand O czym ty mówisz? To jest dokumentacja. Czy możesz wymyślić lepsze miejsce do umieszczenia kluczowych informacji, aby inni mogli je znaleźć niż StackExchange? ;-)
Cort Ammon
43

Mam też tendencję do szukania grepnazwy funkcji / klasy w kodzie, która może dać dodatkowe korzyści, których nie może analizator kodu, na przykład jeśli nazwa jest wymieniona w komentarzu, pliku dokumentacji lub skrypcie. Uruchamiam grep na plikach w drzewie źródłowym i zapisuję wynik w pliku; zwykle wynik zawiera skondensowane informacje: nazwę pliku / ścieżkę, numer linii i linię, w której napotyka się nazwę, co może dać wskazówki, gdzie funkcja / klasa jest wywoływana lub wymieniana bez żadnego znaczenia semantycznego (w przeciwieństwie do analizatora kodu ) i niezależnie od rozszerzeń plików. Zdecydowanie nie jest to najlepsze rozwiązanie, ale miły dodatek do analizy.

piwi
źródło
11
Nie jestem downvoter'em, ale co jeśli podejrzewasz, że sekcja kodu nigdy nie zostanie uruchomiona, nie jest to pełna funkcja lub klasa, ale sekcja metody / funkcji?
Tulains Córdova
9
W zależności od języka może to nie zawsze być niezawodne - niektóre języki umożliwiają dynamiczne wykonywanie wywołań metod. Na przykład php:$object->$variableMethod($arg1, $arg2);
Dezza
9
@ TulainsCórdova To oczywiście nie jest dobre rozwiązanie. Jak w przypadku każdego potencjalnego rozwiązania, użyj go, jeśli ma to sens w danym kontekście. Ale może grepnp. funkcja obejmująca sekcję kodu może dać wskazówki, w jaki sposób funkcja jest używana, a zatem jej zawartość?
piwi
7
„na przykład, jeśli nazwa jest wymieniona w komentarzu, pliku dokumentacji lub skrypcie” - Lub jeśli ktoś używa refleksji do wywołania metody (jeśli używa języka wysokiego poziomu / OO). Choć wciąż niekoniecznie w 100% dokładne, ponieważ niektóre języki obsługują niektóre bardzo tępe sposoby znajdowania i wywoływania metod.
aroth
Jest to pomocne, jeśli masz dostęp do całego możliwego kodu, który mógłby korzystać z tej funkcji. Jeśli jednak kod jest częścią biblioteki, skąd masz wiedzieć, że coś innego, czego nie możesz przeszukać, nie ulegnie uszkodzeniu?
Kevin Shea
30
  • Zidentyfikuj kod, który wygląda na martwy (analiza statyczna itp.).
  • Dodaj komunikat dziennika dla każdego wywołania rzekomo martwego kodu. Jest to łatwe dzięki funkcjom / metodom; z elementami statycznymi, takimi jak stałe, jest to trudniejsze. Czasami możesz po prostu oznaczyć kod jako przestarzały, a środowisko wykonawcze automatycznie zarejestruje komunikat. Pozostaw kod nienaruszony.
    • Po załadowaniu martwego modułu zaloguj komunikat: większość języków ma sposób na uruchomienie inicjalizacji statycznej w czasie ładowania, która może być wspierana przez piggy back.
    • Upewnij się, że komunikat dziennika zawiera rozsądne dane śledzenia stosu, aby zrozumieć, co nazywa się rzekomo martwym kodem.
  • Uruchom zmieniony kod we wszystkich pakietach testowych. Wasze zestawy testowe powinny również testować na specjalne okazje, takie jak przekroczenie północy, przełom kwartału, przełom roku itp. Spójrz na dzienniki, zaktualizuj swoje rozumienie tego, co nie żyje. Należy pamiętać, że testy jednostkowe mogą konkretnie testować martwy kod, podczas gdy nie dotykają go żadne inne testy jednostkowe ani testy integracyjne.
  • Uruchom zmieniony kod produkcyjny na kilka tygodni. Upewnij się, że każdy okresowy proces, taki jak zadania cron ETL przeprowadzane raz w miesiącu, są uruchamiane w tym okresie.
  • Spójrz na dzienniki. Wszystko, co zostało zarejestrowane, w rzeczywistości nie jest martwe. Przejściowe zamknięcie wykresu wywołania w pozostałej części martwego kodu również potencjalnie nie jest martwe, nawet jeśli nie zostało wywołane. Przeanalizuj to. Być może niektóre gałęzie są całkowicie martwe (np. Pracują z interfejsem API wygasłej usługi), a niektóre jeszcze nie. Być może cały moduł / klasa jest ładowany tylko dla zdefiniowanej w nim stałej i można go łatwo nie używać.
  • Wszystko, co pozostało, jest bezpiecznie martwe i można je usunąć.
9000
źródło
Po raz pierwszy wszystko, więc to, że kod nie działał wcześniej, nie oznacza, że ​​nigdy się nie uruchomi, szczególnie jeśli istnieje ścieżka do niego. To powiedziawszy, decyzja o usunięciu czegoś, co można wykonać, jest procesem innym niż ustalenie, czy można go uruchomić.
7
@nocomprende dokładnie to, co się stanie, jeśli ten kod zostanie użyty tylko do funkcji końca roku, a test zostanie uruchomiony w lutym - listopadzie. Nie znajdziesz użycia, nawet jeśli profilowałeś go przez prawie cały rok.
Erik,
1
@ 9000 Teoretycznie zgadzam się, ale starsze systemy są pełne ukrytych zależności, które mogą udaremnić tego rodzaju testy. Aby przykład testowy zadziałał, musisz wiedzieć, że istnieje jakiś rodzaj połączenia czasowego. Jeśli sprzężenie czasowe jest zewnętrzne w stosunku do testowanego kodu, nie zobaczysz go w kodzie i nie wiesz, że sprzężenie czasowe jest czynnikiem. Na przykład kod silosu A jest zainstalowany w GAC . Wiele lat temu silos B poprosił silos A o dodanie ścieżki kodu dla raportu, który należy przeprowadzić z danymi silosu A. cd.
Erik
1
@ 9000 cd. Teraz kod silosu B ma czasowe sprzężenie ze „martwą” ścieżką kodu w kodzie silosu A, co nie jest widoczne po prostu patrząc na kod silosu A. Jak byś przeprowadził test jednostkowy dla tego sprzężenia czasowego, ponieważ sprzężenie czasowe znajduje się tylko w kodzie silosu B, bez znajomości rozmowy z silosem B? Nawet wtedy czas nie jest czynnikiem w twoim kodzie (silos A), więc jak wyglądałby test czasowy?
Erik
2
Czy ktoś pamięta panikę Y2k ? Niektóre kody były poprawne w 2000 roku, ponieważ były błędne dla 2100! Mieliśmy potencjalny problem, który może wystąpić raz na 100 lat lub nawet raz na 400 lat.
Steve Barnes,
16

Oprócz wspomnianych istniejących odpowiedzi można również iteracyjnie usunąć kod w kilku wersjach.

W początkowej wersji można było zrobić ostrzeżenie o wycofaniu z działającym blokiem kodu. W późniejszej wersji można było usunąć blok kodu, ale pozostawić komunikat o błędzie, informujący użytkowników, że funkcja jest przestarzała i nie jest już dostępna. W ostatecznej wersji usunąłbyś blok kodu i wszelkie wiadomości.

Może to pomóc zidentyfikować nieprzewidzianą funkcjonalność bez ostrzeżenia dla użytkowników końcowych. W najlepszym przypadku kod naprawdę nic nie robi, a wszystko to dzieje się tak, że niepotrzebny kod jest przechowywany w kilku wersjach przed usunięciem.

użytkownik3196468
źródło
7
+1. Widziałem tak wiele błędów i problemów z klientami, których można by uniknąć, gdyby programiści (lub ich kierownictwo) byli gotowi rozbić projekt na dwie lub trzy bezpieczne fazy zamiast próbować wrzucić wszystko naraz dowolny termin przed przejściem do następnego projektu.
ruakh 8.03.17
Odpowiada to na pytanie zadane w tytule, ale jeśli mnie zapytacie, pytanie ma zły tytuł. Tytuł pyta, jak usunąć kod, ale w treści chodzi o to, jak ustalić, czy kod można bezpiecznie usunąć. Zdecydowanie odpowiadasz na to pierwsze, ale nie jestem pewien, czy o to OP tak naprawdę pytał.
underscore_d
7

Wiele osób sugeruje, że „bezpieczną” rzeczą jest pozostawienie kodu, jeśli nie możesz udowodnić, że jest nieużywany.

Ale kod nie jest zasobem, ale zobowiązaniem.

O ile nie możesz wyjaśnić, dlaczego jest to ważne i wskazać na jego testy, „bezpieczniejszą” alternatywą może być usunięcie go.

Jeśli nadal nie masz pewności, dodaj przynajmniej testy, aby wykonać kod zombie.

Adrian McCarthy
źródło
Krok 1: Usuń cały kod. Krok 2: odłóż to, co jest potrzebne. Krok 3: powtórz.
1
@Adrian Czy mogę zwrócić uwagę na Knight Capital? en.wikipedia.org/wiki/… - na wypadek, gdybyś chciał wzmocnić swój punkt w odniesieniu do niego. Zasadniczo implodowali, ponieważ jakiś stary kod wrócił do życia i uprzejmie stracił dla nich prawie 500 milionów dolarów. Późniejsze zapytanie koncentrowało się na procedurach ich wydania, ale głównym problemem było „martwa funkcjonalność nie została usunięta”.
SusanW,
6

Możesz użyć przełączników funkcji, aby zmienić ścieżkę wykonania oprogramowania i całkowicie zignorować dany kod.

W ten sposób możesz bezpiecznie wdrożyć swoją zmianę, gdy kod nie jest używany, a przełącznik jest wyłączony. Jeśli zauważysz kilka poważnych błędów związanych z kodem, włącz ponownie przełącznik i sprawdź możliwą ścieżkę do niego.

Takie podejście powinno dać ci pewność, jeśli nie zauważysz żadnych problemów przez dłuższy czas, a także możliwość przywrócenia go bez wdrożenia. Jednak jeszcze lepszym sposobem byłoby zastosowanie dodatkowego rejestrowania i pokrycia testowego na danym obszarze, co dostarczy więcej dowodów na to, czy jest on używany, czy nie.

Przeczytaj więcej o przełącznikach tutaj: https://martinfowler.com/articles/feature-toggles.html

Szarfa
źródło
6

Oczywiście analiza statyczna ... i miło, że nie potrzebujesz żadnych nowych narzędzi; twój kompilator ma wszystkie potrzebne narzędzia do analizy statycznej.

Po prostu zmień nazwę metody (np. Zmień DoSomethingna DoSomethingX), a następnie uruchom kompilację.

Jeśli kompilacja się powiedzie, metoda oczywiście nie jest przez nic używana. Bezpieczne do usunięcia.

Jeśli kompilacja się nie powiedzie, odizoluj wiersz kodu, który ją wywołuje, i sprawdź, jakie ifinstrukcje otaczają to połączenie, aby ustalić, jak je wywołać. Jeśli nie możesz znaleźć żadnego możliwego przypadku użycia danych, który je wyzwala, możesz go bezpiecznie usunąć.

Jeśli naprawdę martwisz się usunięciem, rozważ zachowanie kodu, ale oznaczenie go za pomocą ObsoleteAttribute (lub odpowiednika w twoim języku). Wydaj go w ten sposób dla jednej wersji, a następnie usuń kod, gdy nie zostaną wykryte żadne problemy na wolności.

John Wu
źródło
8
Jest to dobra rada dla języków programowania, które nie mają pełnego dynamicznego / późnego wiązania . Jednak dla tych, którzy mają, jest trudniejsze. I oczywiście - kod może używać tej metody, nawet jeśli nigdy nie jest używana w środowisku produkcyjnym.
eis
Pytanie zakłada rozdzielczość symboli w czasie kompilacji ( więc znalazłeś trochę kodu, który wygląda na zbyteczny. Kompilator tego nie zauważa. ) Nawet późno powiązane funkcje lub obiekty są zazwyczaj definiowane w dwóch miejscach, np. W interfejsie lub pliku mapy, więc kompilacja nadal nie powiedzie się.
John Wu
1
Hmm ... Jestem zaintrygowany, nie sądzę, żeby to drugie, co mówisz, było prawdą. To nie jest na przykład dla javascript waniliowy (bez prekompilacji), ruby, python lub smalltalk, prawda? Jeśli odwołujesz się do rzeczy, które nie istnieją, te błędy występują tylko w czasie wykonywania.
eis
Może nie zgadzamy się co do tego, co rozumie się przez „wiązanie”, co dla mnie oznacza rozdzielczość reprezentacji logicznej na reprezentację fizyczną, np. Symbol na adres. Funkcje JavaScript są przechowywane jako pary znaczników / wartości w obiekcie zawierającym, więc wszelkie pojęcia wiązania wydają się nie być sekwencyjne.
John Wu
1
Tak, naprawdę się nie zgadzamy. Termin ma inne znaczenie dla języków, które nie mają pełnego późnego wiązania (jak wyjaśniłem w pierwszym komentarzu). Kiedy łączysz metody w czasie wykonywania - w językach, które mogą dodawać i usuwać metody podczas pracy - używasz późnego wiązania w sposób, w jaki rozumiem, że należy to zrobić. Dotyczy to języków w stylu ruby ​​/ python / smalltalk i tam nie ma osobnych plików map. Ponieważ szczególnie rubin i python są bardzo powszechnie używane, nie zgadzam się z twoją opinią, co to znaczy „typowo”.
eis
4
  • Chciałbym zacząć korzystać z analizatora kodu, aby sprawdzić, czy to martwy kod
  • Zacząłem sprawdzać moje testy i próbować dotrzeć do kodu. Jeśli nie jestem w stanie dotrzeć do kodu w przypadku testowym, może to być martwy kod
  • Chciałbym usunąć kod i zamiast tego zgłosić wyjątek. W jednym wydaniu wyjątek jest aktywowany, aw drugim wydaniu wyjątek można usunąć. Dla bezpieczeństwa umieść flagę awaryjną (w Javie, właściwość systemową), aby móc aktywować oryginalny kod, gdy klient zauważy wyjątek. Tak więc wyjątek można wyłączyć, a oryginalny kod można aktywować w środowisku produkcyjnym.
Markus Lausberg
źródło
6
-1. Absolutnie nie jest zalecane wprowadzanie tych zmian w kodzie produkcyjnym, symbolu bezpieczeństwa lub nie. Tylko wtedy, gdy jest się pewnym, że dany kod nie jest używany, należy go usunąć w kodzie produkcyjnym. Ten rodzaj bałaganu jest dokładnie powodem, dla którego zawsze powinny istnieć inne środowiska produktywne i testowe.
Johannes Hahn
3
Będzie to działało w fantazji, gdy pokrycie testowe wynosi prawie 90%, ale co, jeśli nie, a masz projekt zawierający 100 000 linii kodu. Właśnie dlatego opisałem, aby spróbować przetestować kod i jeśli żaden test nie jest w stanie dotrzeć do kodu ... może być martwy.
Markus Lausberg,
IMHO Jeśli nie masz wystarczającej pewności, aby całkowicie usunąć kod - nie powinieneś rzucać wyjątków w produkcji - rejestrowanie sugerowane przez OP jest znacznie lepszą alternatywą, osiąga to samo i można uzyskać diagnostykę zamiast polegać na skargach klientów
Sebi
@JohannesHahn Myślę, że masz na myśli „kod produkcyjny”, czyli kod działający w środowisku produkcyjnym.
jpmc26
@ jpmc26 Tak, oczywiście.
Johannes Hahn
3

Usuwanie nieosiągalnego kodu

W statycznym typie języka statycznego zawsze powinieneś wiedzieć, czy kod jest rzeczywiście osiągalny, czy nie: usuń go, skompiluj, jeśli nie ma błędu, to nie był osiągalny.

Niestety, nie wszystkie języki są typowane statycznie i nie wszystkie języki typowane statycznie są oparte na zasadach. Do rzeczy, które mogą pójść nie tak, należą (1) refleksja i (2) nieokreślone przeciążenie.

Jeśli używasz języka dynamicznego lub języka z wystarczająco silnym odzwierciedleniem, że analizowany fragment kodu może być potencjalnie dostępny w czasie wykonywania przez odbicie, nie możesz polegać na kompilatorze. Takie języki to Python, Ruby lub Java.

Jeśli używasz języka z nieprincised przeciążeniem, samo usunięcie przeciążenia może po prostu bez problemu przełączyć rozdzielczość przeciążenia na inne . Niektóre takie języki pozwalają zaprogramować ostrzeżenie / błąd podczas kompilacji związane z użyciem kodu, w przeciwnym razie nie można polegać na kompilatorze. Takie języki to Java (użycie @Deprecated) lub C ++ (użycie [[deprecated]]lub = delete).

Tak więc, chyba że masz szczęście pracować ze ścisłymi językami (przychodzi na myśl Rust), możesz naprawdę strzelać sobie w nogi, ufając kompilatorowi. I niestety pakiety testowe są na ogół niekompletne, więc też niewiele więcej pomocy.

Przejrzyj następną sekcję ...


Usuwanie potencjalnie nieużywanego kodu

Bardziej prawdopodobne jest, że kod jest faktycznie przywoływany, jednak podejrzewasz, że w praktyce gałęzie kodu, które go dotyczą, nigdy nie są pobierane.

W takim przypadku, bez względu na język, kod jest łatwo dostępny i można używać tylko oprzyrządowania w czasie wykonywania.

W przeszłości z powodzeniem stosowałem trójfazowe podejście do usuwania takiego kodu:

  1. Na każdej gałęzi, co do której podejrzewa się, że NIE zostanie podjęta, zaloguj ostrzeżenie.
  2. Po jednym cyklu wyrzuć wyjątek / zwróć błąd po wprowadzeniu określonego fragmentu kodu.
  3. Po kolejnym cyklu usuń kod.

Co to jest cykl Jest to cykl użycia kodu. Na przykład w przypadku wniosku finansowego spodziewałbym się krótkiego cyklu miesięcznego (z wypłatami wynagrodzeń na koniec miesiąca) i długiego cyklu rocznego. W takim przypadku musisz poczekać co najmniej rok, aby sprawdzić, czy ostrzeżenie nigdy nie jest emitowane, aby inwentaryzacja na koniec roku mogła korzystać ze ścieżek kodu, które w innym przypadku nigdy nie byłyby używane.

Mamy nadzieję, że większość aplikacji ma krótsze cykle.

Radzę umieścić komentarz do zrobienia TODO z datą, wskazujący, kiedy przejść do następnego kroku. I przypomnienie w twoim kalendarzu.

Matthieu M.
źródło
1
Właściwie w C ++ możesz oznaczyć podejrzane przeciążenie, [[deprecated]]aby zidentyfikować strony, które wywołują tę wersję; możesz je sprawdzić, aby sprawdzić, czy ich zachowanie się zmieni. Alternatywnie, zdefiniuj swoje przeciążenie jako = delete- w takim przypadku będziesz musiał jawnie rzucić argumenty, aby użyć jednej z pozostałych wersji.
Toby Speight
@TobySpeight: To prawda, a także dobra alternatywa. Pozwól mi to edytować.
Matthieu M.
„Doktorze, to boli, kiedy to robię”. „Cóż , nie rób tego! ” Nie używaj metod, które uniemożliwiają zarządzanie kodem.
2

Usuwanie kodu z produkcji przypomina sprzątanie domu. W momencie, gdy wyrzucisz przedmiot ze strychu, twoja żona zabije cię następnego dnia za wyrzucenie prezentu na powrót siostrzeńca trzeciej pradziadki od sąsiada, który zmarł w 1923 roku.

Poważnie, po pobieżnej analizie przy użyciu różnych narzędzi, o których wszyscy już wspominali, a także po zastosowaniu wspomnianej już metody stopniowej rezygnacji, pamiętaj, że możesz zniknąć z firmy, kiedy nastąpi faktyczne usunięcie. Pozostawienie kodu i zarejestrowanie jego wykonania oraz upewnienie się, że wszelkie powiadomienia o jego wykonaniu zostaną zdecydowanie przekazane Tobie (lub Twoim następcom i cesjonariuszom) jest niezbędne.

Jeśli nie zastosujesz się do tego podejścia, prawdopodobnie zostaniesz zabity przez swoją żonę. Jeśli zachowasz kod (jak każdy pamiątkowy), możesz uspokoić sumienie, że prawo Moore'a przychodzi ci na ratunek, a koszt miejsca na śmieci używanego przez kod, który wydaje się być „kamieniem w moim bucie”, zmniejsza każdy rok i nie ryzykujesz, że staniesz się centrum wielu plotek na temat chłodnicy wodnej i dziwnych spojrzeń na korytarzach.

PS: Aby wyjaśnić w odpowiedzi na komentarz. Pierwotne pytanie brzmi: „Jak bezpiecznie usunąć…” Oczywiście, w mojej odpowiedzi założono, że kontrola wersji. Zakłada się, podobnie jak kopanie w koszu w celu znalezienia wartościowego. Żadne ciało, które ma jakikolwiek sens, nie wyrzuca kodu, a kontrola wersji powinna być częścią rygorów każdego dewelopera.

Problem dotyczy kodu, który może być zbędny. Nigdy nie możemy wiedzieć, że fragment kodu jest zbyteczny, chyba że możemy zagwarantować, że 100% ścieżek wykonania go nie osiągnie. Zakłada się, że jest to wystarczająco duże oprogramowanie, że gwarancja jest prawie niemożliwa. I chyba, że ​​źle przeczytam pytanie, cały ten wątek rozmowy jest istotny tylko dla przypadków podczas produkcji, w których może zostać wywołany usunięty kod, a zatem mamy problem z czasem wykonania / produkcją. Kontrola wersji nie oszczędza nikogo z powodu błędów produkcyjnych, więc komentarz na temat „Kontrola wersji” nie ma związku z pytaniem lub moim pierwotnym punktem, tj. Odpowiednio przestarzałym w bardzo długim okresie czasu, jeśli naprawdę musisz, inaczej nie „

IMHO, komentarz jest zbyteczny i może zostać usunięty.

LMSingh
źródło
12
Gdybym miał odpowiednik kontroli wersji , sprzeciw mojej żony można łatwo cofnąć. Podobnie jest z usuwaniem kodu: to nie jest zabójstwo. Repozytorium nigdy nie jest wyrzucane.
JDługosz
Myślałem, że programowanie obiektowe powinno zawęzić zakres, w którym można wywołać metodę. Powinno to prowadzić do kodu łatwiejszego do zrozumienia i zarządzania, prawda? W przeciwnym razie dlaczego robimy to w ten sposób?
@nocomprende Nigdzie w tym wątku nie wskazano, że dyskusja ogranicza się wyłącznie do projektowania / języków OOP. Nie jestem pewien, dlaczego uważasz, że to jest istotne.
underscore_d
1

Może kompilator to zauważył, po prostu nie robi to zamieszania.

Uruchom kompilację z pełną optymalizacją kompilatora, dostosowaną do rozmiaru. Następnie usuń podejrzany kod i ponownie uruchom kompilację.

Porównaj pliki binarne. Jeśli są identyczne, kompilator zauważył i po cichu usunął kod. Możesz go bezpiecznie usunąć ze źródła.

Jeśli pliki binarne są różne ... to nie jest jednoznaczne. To może być jakaś inna zmiana. Niektóre kompilatory zawierają nawet datę i godzinę kompilacji w pliku binarnym (i być może można je skonfigurować!)

Emilio M. Bumachar
źródło
1
Nie wszystkie języki są skompilowane, nie wszystkie kompilatory mają optymalizację i nie wszystkie optymalizacje są równe, wiele nie usuwa nieprzywołanego kodu na wypadek, gdyby kod był z jakiegoś powodu. Jeśli kod znajduje się we współdzielonej bibliotece / DLL, nie zostanie usunięty.
Steve Barnes,
1
@ SteveBarnes Cóż, jeśli nie robisz LTO i nie łączysz wszystkiego statycznie, czeka Cię ból. To jest pewne.
Navin
1

Tak naprawdę ostatnio spotkałem się z tą dokładną sytuacją, stosując metodę o nazwie „deleteFoo”. Nigdzie w całej bazie kodu nie znaleziono tego łańcucha poza deklaracją metody, ale mądrze napisałem wiersz dziennika na górze metody.

PHP:

public function deleteFoo()
{
    error_log("In deleteFoo()", 3, "/path/to/log");
}

Okazuje się, że kod został użyty! Niektóre metody AJAX wywołują „method: delete, elem = Foo”, która jest następnie łączona i wywoływana za pomocą call_user_func_array () .

W razie wątpliwości zaloguj się! Jeśli upłynął wystarczający czas bez wypełnienia dziennika, rozważ usunięcie kodu. Ale nawet jeśli usuniesz kod, zostaw dziennik na miejscu i komentarz z datą, aby ułatwić znalezienie zatwierdzenia w Git dla faceta, który musi go zachować!

dotancohen
źródło
Istnieją biblioteki dla PHP i Perla o nazwie Tombstone (php: github.com/scheb/tombstone ), które mogą być przydatne tutaj.
Alister Bulman,
0

Skorzystaj z grepjakichkolwiek dostępnych wcześniej narzędzi, możesz znaleźć odniesienia do kodu. (Nie pierwotnie moja instrukcja / rada.)

Następnie zdecyduj, czy chcesz zaryzykować usunięcie kodu, czy też moja sugestia może się przydać:

Możesz zmodyfikować funkcję / blok kodu, aby zapisać, że został użyty w pliku dziennika. Jeśli taka wiadomość nie zostanie „kiedykolwiek” zapisana w pliku dziennika, prawdopodobnie możesz usunąć kod logowania.

Dwa problemy:

  1. Pobieranie dziennika od innych użytkowników. Być może możesz ustawić globalną flagę, która spowoduje awarię programu przy wyjściu i uprzejmie poprosić użytkownika o przesłanie dziennika błędów.

  2. Śledzenie dzwoniącego. W niektórych językach wysokiego poziomu (np. Python) można łatwo uzyskać informacje zwrotne. Ale w skompilowanych językach musisz zapisać adres zwrotny w dzienniku i wymusić zrzut pamięci przy wyjściu (punkt 1).
    W C powinno to być dość proste: użyj bloku warunkowego (np. #Ifdef ... #endif), aby użyć tego tylko wtedy, gdy wiesz, że zadziała (i przetestowałeś, że działa), i po prostu przeczytaj adres zwrotny ze stosu (może wymagać lub nie wbudowanego języka asemblera).

Zapisanie argumentów w pliku dziennika może, ale nie musi być przydatne.

Oskar Skog
źródło
0

Jeśli wiesz, co robi otaczający go kod, przetestuj go, aby sprawdzić, czy nie działa. Jest to najprostszy i najbardziej opłacalny sposób na refaktoryzację fragmentu kodu, nad którym pracujesz, i należy to zrobić w każdym razie w celu opracowania wszelkich zmian w kodzie, nad którym pracujesz.

Jeśli natomiast refaktoryzujesz fragment kodu, w którym nie wiesz, co on robi, nie usuwaj go . Zrozum to najpierw, a następnie refaktoryzuj, jeśli określisz, że jest bezpieczny (i tylko jeśli możesz ustalić, że refaktoryzacja byłaby właściwym wykorzystaniem twojego czasu).

Teraz mogą zaistnieć pewne okoliczności (wiem, że sam na to wpadłem), w których kod „martwy” nie jest w rzeczywistości powiązany z niczym . Takich jak biblioteki kodu opracowane przez kontrahenta, które nigdy nie zostały nigdzie zaimplementowane, ponieważ stały się przestarzałe natychmiast po dostarczeniu aplikacji (dlaczego tak, mam na myśli konkretny przykład, skąd wiesz?).

Osobiście skończyłem z usuwaniem całego tego kodu, ale jest to ogromne ryzyko, że nie polecam lekceważyć - w zależności od tego, ile kodu ta zmiana może potencjalnie wpłynąć (ponieważ zawsze istnieje możliwość, że coś przeoczyłeś) powinien być wyjątkowo ostrożny i przeprowadzać agresywne testy jednostkowe, aby ustalić, czy usunięcie tego starożytnego starszego kodu spowoduje uszkodzenie aplikacji.

Biorąc to wszystko pod uwagę, prawdopodobnie nie warto tracić czasu ani rozsądku, aby próbować usunąć taki kod. Nie, chyba że stanie się poważnym problemem z twoją aplikacją (na przykład niemożność ukończenia skanowania bezpieczeństwa z powodu rozdęcia aplikacji ... dlaczego tak, wciąż mam na myśli przykład), więc nie poleciłbym go, dopóki nie dotrzesz taka sytuacja, w której kod naprawdę zaczął gnić w imponujący sposób.

Krótko mówiąc - jeśli wiesz, co robi kod, najpierw go przetestuj. Jeśli nie, prawdopodobnie możesz zostawić to w spokoju. Jeśli wiesz, że nie możesz tego zostawić w spokoju, poświęć swój czas na agresywne przetestowanie zmiany przed jej wdrożeniem.

Zibbobz
źródło
0

Moje podejście, które zawsze zakładałem, że jest w tym przypadku standardem branżowym, jak dotąd nie zostało wspomniane:

Zdobądź drugą parę oczu.

Istnieją różne rodzaje „nieużywanego kodu”, a w niektórych przypadkach usunięcie jest właściwe, w innych przypadkach należy je przefiltrować, a w innych przypadkach ucieka się tak daleko, jak to możliwe i nigdy nie oglądać się za siebie. Musisz dowiedzieć się, który to jest, i aby to zrobić, najlepiej połączyć się z drugą - doświadczoną! - programista, który bardzo dobrze zna podstawy kodu. To znacznie zmniejsza ryzyko niepotrzebnej pomyłki i pozwala wprowadzić niezbędne ulepszenia, aby utrzymać bazę kodu w stanie umożliwiającym utrzymanie. A jeśli tego nie zrobisz, w pewnym momencie zabraknie Ci programistów, którzy dobrze znają bazę kodu.

Widziałem też podejście do rejestrowania - ściślej mówiąc, widziałem kod rejestrujący: jeszcze więcej martwego kodu, który pozostawiono tam przez 10 lat. Podejście do logowania jest całkiem przyzwoite, jeśli mówisz o usunięciu całych funkcji, ale nie, jeśli usuwasz tylko niewielki fragment martwego kodu.

Piotr
źródło
0

Prosta odpowiedź na twoje pytanie brzmi: nie możesz bezpiecznie usunąć kodu, którego nie rozumiesz, co obejmuje również niezrozumienie, jak się nazywa. Będzie pewne ryzyko.

Będziesz musiał ocenić, jak najlepiej ograniczyć to nieuniknione ryzyko. Inni wspominali o logowaniu. Rejestrowanie może oczywiście udowodnić, że jest używane, ale nie może udowodnić, że nie jest używane. Ponieważ głównym problemem związanym z martwym kodem jest to, że jest on utrzymywany, być może uda ci się dodać komentarz mówiący, że jest to podejrzany martwy kod, i nie zajmować się nim.

Jeśli kod znajduje się pod kontrolą źródła, zawsze możesz dowiedzieć się, kiedy został dodany i ustalić, jak został wtedy wywołany.

W końcu albo to zrozumiesz, zostaw w spokoju, albo zaryzykuj.

jmoreno
źródło
0

Jeśli twórca danego kodu jest nadal dostępny, zapoznaj się z jego usunięciem . Również przeczytać komentarze w kodzie .

Otrzymasz właściwe wyjaśnienie, dlaczego ten kod został dodany i dla jakiej funkcji służy. Może z łatwością stanowić wsparcie dla konkretnego przypadku użycia, a nawet może nie mieć wystarczającej wiedzy specjalistycznej, aby ocenić, czy tak naprawdę nie jest to wymagane. W skrajnym przypadku można usunąć wszystkie bezpłatne instrukcje z programu C i nie zauważyć niczego złego (zabraknie pamięci może potrwać kilka minut).

To naprawdę zła kultura, kiedy autor kodu siedzi obok ciebie, ale nie widzisz powodu do rozmowy, ponieważ jesteś pewien, że on po prostu pisze zły kod, a twój jest tak doskonały. I, oczywiście, po prostu pisze losowe słowa w komentarzach, nie trzeba tracić czasu na czytanie.

Jednym z najważniejszych powodów napisania komentarza w kodzie jest wyjaśnienie DLACZEGO został on zaimplementowany w ten sposób. Możesz nie zgodzić się z rozwiązaniem, ale musisz wziąć pod uwagę problem.

Dyskusja z autorem może również ujawnić, że kod jest historyczną pozostałością czegoś usuniętego lub nigdy nieukończonego, więc można go bezpiecznie usunąć.

h22
źródło
-1

Zdecydowanie zgadzam się z pierwszą uwagą Markusa Lausberga: Kod należy zdecydowanie przeanalizować za pomocą narzędzi. Mózgom małp nie można ufać przy tego rodzaju zadaniach !!

Większość standardowych języków programowania można stosować w IDE, a każde IDE, które warto nazywać, ma funkcję „znajdź dla wszystkich zastosowań”, która jest dokładnie właściwym narzędziem do tego rodzaju sytuacji. (Na przykład Ctrl + Shift + G w Eclipse)

Oczywiście: narzędzia analizatora statycznego nie są idealne. Należy zdawać sobie sprawę z bardziej skomplikowanych sytuacji, w których decyzja o wywołaniu danej metody ma miejsce tylko w czasie wykonywania i nie wcześniej (wywołania przez odbicie, wywołania funkcji „ewaluacyjnych” w językach skryptowych itp.).

Johannes Hahn
źródło
5
Może mózgi małp nie powinny pisać kodu? Słowonietknięte ludzkimi rękami ” było frazą marketingową. Zawsze wyobrażałem sobie ręce goryla.
3
(Mózg małpy też napisał narzędzia) (
1
Myślę, że oboje nie rozumiecie mojego punktu. Wyraźnie powiedziałem, że nie ma gwarancji, że statyczna analiza kodu znajdzie każde wywołanie metody lub klasy. Jest to, jak mówi na puszce, analiza statyczna. Nie można i nie należy oczekiwać, aby znaleźć dynamiczne wywołania poprzez odbicie, eval () itp. Chodzi mi o to, że statyczna analiza kodu powinna być uważana za niezbędny warunek wstępny usunięcia fragmentu kodu. A im większy projekt, tym mniej prawdopodobne jest, że ludzki mózg jest nawet w stanie przeprowadzić tę analizę, a co dopiero zrobić to poprawnie ...
Johannes Hahn
1
... statyczna analiza kodu to żmudna, powtarzalna praca, którą najlepiej wykonać komputer. Dokładnie takie zadanie, które może być niezawodnie wykonane przez komputery, ale jest wyjątkowo niewiarygodne, gdy wykonuje je małpi mózg. To ostatnie powinno być używane tylko do rzeczy, których komputer nie może zrobić, takich jak znalezienie tych trudnych refleksji i wywołań eval.
Johannes Hahn
1
Znowu brakuje mi sensu. Nawet jeśli narzędzia nie są idealne, ponieważ zostały napisane przez mózg małpy (i nie jestem nawet pewien, czy powinienem to przyznać! Obliczanie statycznych hierarchii wywołań nie jest takie trudne), nadal są one znacznie lepsze niż ręczne wykonywanie zadań. To nie jest sytuacja typu „wszystko albo nic”. 99% niezawodne narzędzie jest nadal lepsze od mózgu, który może zaczyna się od 99%, ale szybko spada do znacznie niższych prędkości po pierwszych tysiącach wierszy kodu.
Johannes Hahn,
-1

Dwie możliwości:

  1. Masz ponad 99% pokrycia testowego : Usuń kod i skończ z nim.
  2. Nie masz wiarygodnego pokrycia testowego: odpowiedzią jest dodanie ostrzeżenia o rezygnacji . To znaczy, dodajesz kod do tego miejsca, które robi coś rozsądnego (na przykład zaloguj ostrzeżenie w dobrze widocznym miejscu, które nie koliduje z codziennym użytkowaniem Twojej aplikacji). Następnie gotuj na wolnym ogniu przez kilka miesięcy / lat. Jeśli nigdy nie zdarzyło się to, zastąpienie przestarzałości czymś, co rzuca wyjątek. Niech to potrwa chwilę. Na koniec usuń wszystko całkowicie.
AnoE
źródło
2
wydaje się, że nie oferuje to nic istotnego w stosunku do punktów przedstawionych i wyjaśnionych w poprzednich 17 odpowiedziach. Omówiono już tam zarówno zasięg, jak i ostrzeżenie o
rezygnacji
@gnat, masz rację. Kiedy po raz pierwszy zeskanowałem odpowiedzi, z jakiegoś powodu myślałem, że widziałem kilka takich, które na szczycie nie dotarły krótko i zwięźle do tych dwóch punktów. W środku odpowiedzi jest kilka takich.
AnoE,