Jak przekonać kolegów z drużyny, że nie powinniśmy ignorować ostrzeżeń kompilatora?

51

Pracuję nad ogromnym projektem (bardziej jak splątaną kombinację dziesiątek mini projektów, których nie można łatwo rozdzielić z powodu złego zarządzania zależnościami, ale to inna dyskusja) w Javie za pomocą eclipse. Wyłączyliśmy już wiele ostrzeżeń w ustawieniach kompilatora, a projekt nadal zawiera ponad 10 000 ostrzeżeń.

Jestem wielkim zwolennikiem próby rozwiązania wszystkich ostrzeżeń, naprawienia wszystkich, jeśli to możliwe, a dla tych, które są sprawdzane i uważane za bezpieczne, pomijaj je. (To samo dotyczy mojej religijnej obsesji na punkcie oznaczania wszystkich zaimplementowanych / zastąpionych metod jako @Override). Moim największym argumentem jest to, że generalnie ostrzeżenia pomagają znaleźć potencjalne błędy podczas kompilacji. Może na 99 na 100 ostrzeżeń ostrzeżenia są nieznaczące, ale myślę, że drapanie się w głowie, które oszczędza po raz pierwszy, że zapobiega poważnemu błędowi, wszystko jest tego warte. (Moim drugim powodem jest mój pozorny OCD z czystością kodu).

Jednak wielu moich kolegów z drużyny nie przejmuje się tym. Czasami naprawiam ostrzeżenia, gdy się na nie natknę (ale wiesz, że to trudne, kiedy dotykasz kodu napisanego przez współpracownika). Teraz, gdy dosłownie jest więcej ostrzeżeń niż klas, zalety ostrzeżeń są bardzo zminimalizowane, ponieważ gdy ostrzeżenia są tak powszechne, nikt nie będzie zawracał sobie głowy ich analizowaniem.

Jak mogę przekonać moich kolegów z drużyny (lub ich moce), że ostrzeżenia należy rozwiązać (lub je stłumić po pełnym zbadaniu)? Czy powinienem przekonać się, że jestem szalony?

Dzięki

(PS Zapomniałem wspomnieć o tym, co ostatecznie skłoniło mnie do opublikowania tego pytania, ponieważ z przykrością zauważyłem, że naprawiam ostrzeżenia wolniej niż się pojawiają)


źródło
5
Wszystkie rodzaje: rawtype, nieużywany import, nieużywana zmienna, nieużywane metody prywatne, niepotrzebne pomijanie, niesprawdzone, niepotrzebne rzutowanie, niepotrzebny warunek (zawsze być prawdziwy lub fałszywy), nieprzypisane zastąpione metody, odniesienie do przestarzałej klasy / metod itp.
1
«Chcę zacząć stosować statyczną analizę„ kodu ”do map gier i traktować ostrzeżenia jako błędy. »Ostatni cytat z John Carmack. Jeśli jesteś szalony, jesteś z siebie. W rzeczywistości jesteśmy nas troje.
deadalnix
1
@RAY: Są to ostrzeżenia z analizy kodu Eclipse, nie jestem pewien, czy można je wszystkie zdobyć z wanilii javac.
yatima2975
4
ale wiesz, że jest to trudne, gdy dotykasz kodu napisanego przez współpracownika - jeśli w twoim miejscu pracy występują tego rodzaju problemy terytorialne, uważam to za wielką czerwoną flagę.
JSB
1
@deadalnix: będąc facetem w C ++, mam tylko jeden sposób na robienie rzeczy -Wall -Wextra -Werror(tj. aktywowanie większości dostępnych ostrzeżeń, traktowanie ich wszystkich jako błędów). Zaćmienie C ++ jest jednak prawie bezużyteczne: /
Matthieu M.

Odpowiedzi:

37

Możesz zrobić dwie rzeczy.

  1. Wskaż, że istnieją ostrzeżenia z jakiegoś powodu. Autorzy kompilatorów nie umieszczają ich, ponieważ są wredni. Ludzie z naszej branży są na ogół pomocni. Wiele ostrzeżeń jest pomocnych.

  2. Zbierz historię spektakularnych awarii wynikających z ignorowania ostrzeżeń. Wyszukiwarka „Zwróć uwagę na ostrzeżenia kompilatora” zwraca pewne anegdoty.

Twoja obsesja @Overridenie jest „obsesją”. To jest dobra rzecz. Czy kiedykolwiek źle napisałeś nazwę metody?

Ray Toal
źródło
8
Chciałbym dodać, że nie możesz sprawić, by inni się tym przejmowali, więc bądź pragmatyczny i dobrze wybieraj swoje bitwy.
Daniel Werner
@Daniel: smutny, ale prawdziwy. Jeśli ich to nie obchodzi, nawet jeśli wiedzą (lub przynajmniej wierzą ), że masz rację, to nie jest to zadanie z różową perspektywą.
Joachim Sauer
2
Jest wiele razy, kiedy ostrzeżenia są w rzeczywistości tylko ostrzeżeniami. Możesz być czujny, ale często wolę spędzać ten czas na pisaniu przypadków testowych, które są znacznie bardziej specyficzne dla twojej bazy kodu.
ghayes
Myślę, że OP chce, aby współpracownicy przestali być lekceważący. To prawda, że ​​wiele ostrzeżeń nie musi być adresowanych, i nie powinny być wszystkie! Ale bycie złym i pozwolenie, aby 10 000 z nich wkradło się do bazy kodu, nie jest dobrą rzeczą. Ostrzeżenia należy przejrzeć, rozważyć, a jeśli nie mają zastosowania, można je wyłączyć lub zastosować adnotację Pomiń.
3
Niedawno uporządkowałem niektóre ostrzeżenia w projekcie open source i znalazłem wyraźny błąd, który został zgłoszony za pomocą ostrzeżenia: if (error = 0)zamiast if (error == 0). Ponadto wiele ostrzeżeń znacznie ułatwia znajdowanie błędów kompilatora bez konieczności przechodzenia przez szereg ostrzeżeń.
Hugo,
12

Odpowiednie czytanie tutaj . C ++, ale nadal aktualne. Szczególnie podoba mi się ten przykład (komentarz do kodu jest mój):

int searchArray(int to_search[], int len, int to_find)
{
    int i;
    for( i = 0; i < len; ++i )
    {       
        if ( to_search[i] == to_find )
        {
            return i;
        }
    }
    // should be returning designated 'not found' value (0, -1, etc)
}

Często ostrzeżenie będzie oznaczało różnicę między awarią aplikacji z błędem, która faktycznie pomoże wyśledzić wadę projektową i naprawić ją (np. Bezpieczne rzutowanie czcionek) lub aplikację przyjmującą założenia, a następnie zachowującą się nieprawidłowo lub błędny sposób (co miałoby miejsce w przypadku niebezpiecznego rzutowania). Podobnie wskazują również cruft lub martwy kod, który można usunąć (nieosiągalne bloki kodu, nieużywane zmienne), co można uznać za optymalizację kodu lub przypadki nieuwzględnione w testach, jak powyższy przykład dla C ++. Zauważ, że ten przykład powoduje błąd kompilacji w Javie.

Tak więc naprawianie ostrzeżeń ma (przynajmniej) kilka zalet dla zarządzania:

  1. Mniejsza szansa, że ​​kod zachowuje się niepoprawnie w przypadku danych wprowadzanych przez użytkownika, co dla osób zajmujących wyższe stanowiska dotyczące wizerunku firmy i sposobu, w jaki obchodzi się z danymi klientów, powinno być wielką sprawą (tm). Awaria, aby uniemożliwić użytkownikowi zrobienie czegoś głupiego, jest lepsza niż nie awaria i pozwolenie mu to zrobić.
  2. Jeśli chcą przetasować personel, ludzie mogą wejść do bazy kodów wolnej od ostrzeżeń (i nie wystraszyć się ani nie narzekać tyle ...). Może to zmniejszy ilość narzekania lub opinie na temat możliwości utrzymania lub jakości wkładu Drużyny X w Projekt Y.
  3. Optymalizacja kodu poprzez usunięcie ostrzeżeń kompilatora, bez znaczącej poprawy czasu działania, poprawi liczne wyniki testów:
    • Wszelkie wyniki pokrycia kodu prawdopodobnie wzrosną.
    • Testowanie granic i niepoprawnych przypadków testowych prawdopodobnie zakończy się powodzeniem częściej ( między innymi ze względu na wyżej wspomniane bezpieczne rzutowanie typów).
    • Gdy przypadek testowy zawiedzie, nie musisz zastanawiać się, czy wynika to z jednego z ostrzeżeń kompilatora w tym pliku źródłowym.
    • Łatwo jest argumentować, że zero ostrzeżeń kompilatora jest lepsze niż tysiące. Żadne ostrzeżenia lub błędy nie mówią o jakości kodu, więc możesz argumentować, że jakość kodu poprawia się, im mniej ostrzeżeń.
  4. Jeśli nie masz ostrzeżeń kompilatora i wprowadzono jedno lub więcej, o wiele łatwiej jest ustalić, kto spowodował ich pojawienie się w bazie kodu. Jeśli masz politykę „bez ostrzeżeń”, która pomogłaby im zidentyfikować osoby, które nie wykonują prawidłowo swojej pracy? Pisanie kodu nie polega tylko na tym, aby coś działało, ale na tym, aby działało dobrze .

Zauważ, że wciąż jestem młodszym programistą, który niewiele wie o zarządzaniu projektami, więc jeśli powiedziałem coś nie tak, popraw moje myślenie i daj mi szansę na edycję, zanim zdążysz głosować za mną :)

darvids0n
źródło
2
bardzo przemyślana odpowiedź. Dziękuję Ci. Podany przykład będzie jednak błędem, a nie ostrzeżeniem w Javie. :)
@RAY: Ach, w porządku. Minął rok, odkąd stworzyłem programistę Java, więc musiałem coś przeoczyć. Zmienię mój wpis, aby o tym wspomnieć. Dzięki!
darvids0n
Minęło trochę czasu odkąd zrobiłem C ++. Co zwraca ta funkcja, jeśli dojdziesz do komentarza na końcu? Czy to 0? Nieokreślone zachowanie?
MatrixFrog,
1
@MatrixFrog: Undefined. Może być dowolną int, która spowoduje różnego rodzaju nieprzyjemności. Cytat z tej odpowiedzi: „ C ++ 03 §6.6.3 / 2: Wypływanie końca funkcji jest równoznaczne ze zwrotem bez wartości; powoduje to niezdefiniowane zachowanie funkcji zwracającej wartość.
darvids0n
7

W przeszłości pracowałem nad projektem o podobnych cechach. Ten w szczególności został pierwotnie napisany w Javie 1.4. Gdy pojawi się Java 5 z ogólnymi wersjami, można sobie wyobrazić liczbę ostrzeżeń generowanych przez kompilator przy każdym użyciu interfejsu API kolekcji.

Rozpoczęcie używania leków generycznych zajęłoby sporo czasu, aby pozbyć się wszystkich ostrzeżeń. Jest to czynnik, który należy wziąć pod uwagę, ale gdy musisz przekonać kogoś (szczególnie swojego kierownika), że wymaga on naprawy, będziesz potrzebować twardych danych, takich jak

czas poświęcony na błędy w starszych lub niestandardowych kodach (standard jest standardem kodowania twojego projektu), których można było uniknąć.

Możesz nadal przedstawiać rachunek, który pokazuje, w jaki sposób tracisz czas i pieniądze, ignorując „pewne” ostrzeżenia, a ktoś wpadnie na ten pomysł. Najważniejsze jest to, że nie wszystkie ostrzeżenia są warte obejrzenia, przynajmniej nie od razu; mówiąc prościej, należy ustalić priorytet ostrzeżeń, które należy natychmiast rozwiązać. Na początek rozważ te związane z błędami, które widzisz w swoim projekcie.

Możesz również robić notatki w narzędziu do śledzenia błędów, kiedy naprawiasz błędy, że wspomnianego błędu można było uniknąć, nie ignorując ostrzeżenia (a może uruchamiając PMD lub FindBugs, jeśli masz CI lub system kompilacji). Dopóki istnieje wystarczająca liczba błędów, które można naprawić, słuchając ostrzeżeń kompilatora, twój punkt widzenia na temat ostrzeżeń kompilatora byłby ważny. W przeciwnym razie jest to decyzja biznesowa i zwykle nie warto poświęcać czasu na walkę.

Vineet Reynolds
źródło
Tak, dokładnie. Myślę, że początkowa eksplozja liczby wyjątków miała miejsce podczas przejścia 1.4-> 1.5 i od tego czasu nikt nie zwracał już uwagi na ostrzeżenia, ponieważ jest ich po prostu zbyt wiele ...
2

Jeśli ci się powiedzie, a twój zespół postanowi przestrzegać ostrzeżeń, powinieneś podejść krok po kroku. Nikt nie może i nie będzie 10000 ostrzeżeń jednocześnie. Możesz więc wybrać te najważniejsze (te, które są błędami z dużym prawdopodobieństwem) i wyłączyć te mniej znaczące. Jeśli są naprawione, ponownie zwiększ poziom ostrzeżenia. Dodatkowo możesz zacząć od FindBugs, który ostrzega przed kodem, który prawie zawsze jest błędem.


źródło
2
Lub skup się na naprawianiu ostrzeżeń, gdy spojrzysz na kod (nowe klasy nie powinny mieć ostrzeżeń, wszystkie zmodyfikowane klasy powinny mieć mniej ostrzeżeń).
Przywróć Monikę
Poważne pytanie: skąd wiesz, które z nich najprawdopodobniej spowodują rzeczywiste błędy?
MatrixFrog,
1

Całkowicie się z tobą zgadzam, najlepszą praktyką jest jak najczystsze czyszczenie ostrzeżeń kompilacji. Wspomniałeś, że twój zespół używa Eclipse jako narzędzia programistycznego. Eclipse to bardzo dobre narzędzie, które pomaga oczyścić kod i zapewnić spójność stylu kodu.

  1. zdefiniuj „Zapisz akcję” dla każdego projektu Java, na przykład formatuj kod, nieużywane zmienne lokalne, organizuj importy (myślę, że nikt nie ma zastrzeżeń co do tego rodzaju czyszczenia).
  2. zdefiniuj opcje kompilacji specyficzne dla projektu dla każdego projektu. Na przykład, poziom kompilacji (jeśli twój kod musi być zgodny z 1.4, aby wyczyścić ostrzeżenia związane z ogólnym), przypisanie nie ma wpływu (np. „X = x”) i tak dalej. Ty i twój kolega z drużyny możecie się zgodzić na te elementy, a Eclipse zgłosi je jako „Błąd” w fazie rozwoju po uzyskaniu zgody na błąd kompilacji.

Eclipse utworzy niektóre pliki właściwości dla tych preferencji w folderze .settings, możesz skopiować je do innych projektów Java i sprawdzić je w SCM jako część kodu źródłowego.

Możesz sprawdzić kod Eclipse, aby zobaczyć, jak robią to deweloperzy Eclipse.

Laska
źródło
1

Istnieją dwa sposoby na pozbycie się wszystkich ostrzeżeń (i zgadzam się, że może ukryć subtelny błąd):

  1. Przekonaj cały zespół, że jest to najlepsza rzecz i poproś, aby to naprawili.
  2. Przekonaj swojego szefa, że ​​jest to najlepsza rzecz, i uczyń to obowiązkowym. Następnie muszą to naprawić.

Uważam, że 1 nie jest już w twoim przypadku wykonalny. Prawdopodobnie zajmie to tak dużo czasu ze względu na liczbę ostrzeżeń, które pojawią się na wydajności, więc kierownictwo i tak będzie musiało wiedzieć.

Tak więc proponuję: weź to z szefem, przekonaj go, że to tykająca bomba, i niech to oficjalna polityka.

Thorbjørn Ravn Andersen
źródło
1

Powiedziałbym im po prostu, że większość z nich to nieistotne ostrzeżenia i ważne jest, abyśmy usunęli je z listy. Aby nie przegapić prawdziwego znaczącego ostrzeżenia w tłumie, jak to się dzieje!

WinW
źródło
Dokładnie. Ważne jest, aby się ich pozbyć, ponieważ są nieistotne.
MatrixFrog,
0

Będziesz potrzebował dużo cierpliwości, próbując ich przekonać, usunięcie ostrzeżeń podczas programowania w parach pomoże innym nabrać nawyku. Zaproponuj, aby przeczytali „Efektywna Java” pozycja 24: Wyeliminuj niesprawdzone ostrzeżenia (dla zbioru ogólnego). I prawdopodobnie zignoruj ​​wiele przypadków, w których ludzie nie będą postępować zgodnie z twoją radą;)

Mehul Lalan
źródło
0

Skutecznym podejściem byłoby tutaj skonfigurowanie serwera Continuous Integration , który automatycznie buduje projekt i uruchamia testy za każdym razem, gdy ktoś sprawdza kod. Jeśli jeszcze tego nie używasz, powinieneś i nie jest trudno przekonać innych o korzyściach płynących z tego.

  1. Przekonaj kierownictwo / kierowników zespołów o korzyściach płynących z tego (zapobieganie wdrażaniu złych wersji, wczesne wykrywanie błędów, szczególnie tych, które przypadkowo wpływają na inne części oprogramowania, utrzymywanie testów regresji, wczesne i częste testowanie itp.)

  2. Zainstaluj serwer CI z pozytywnym wynikiem wszystkich testów (jeśli nie masz testów, napisz szybki, który przejdzie, aby wszyscy widzieli, że jest zielony).

  3. Skonfiguruj wiadomości e-mail lub inne powiadomienia o stanie każdej kompilacji. Ważne jest tutaj, aby podać, kto sprawdził w kodzie, jaka była zmiana (lub link do zatwierdzenia) oraz status + wyjście kompilacji. Jest to ważny krok, ponieważ powoduje widoczność w całym zespole zarówno pod względem sukcesów, jak i porażek.

  4. Zaktualizuj serwer CI, aby testy zakończyły się niepowodzeniem, jeśli pojawią się jakieś ostrzeżenia. Kierownictwo i kierownicy zespołu postrzegają to jako systematyczne zwiększanie dyscypliny zespołu i nikt nie chce być odpowiedzialny za wszystkie wiadomości e-mail o niepowodzeniach.

  5. (opcjonalnie) bądź miły i naukowy dzięki dodaniu widocznych pulpitów nawigacyjnych, lamp lawowych, migających świateł itp., aby wskazać status kompilacji i kto ją zepsuł / naprawił.

Ben Taitelbaum
źródło