Jaki jest cel przeglądu kodu

76

Próbuję sprzedać mojej organizacji wartość przeglądów kodu. Pracowałem w kilku miejscach, w których byli zatrudnieni. Widziałem, jak są przyzwyczajeni do wyborów stylizacji i decyzji funkcjonalnych, i widziałem, że były używane jedynie jako kontrola, aby upewnić się, że nic niebezpiecznego nie zostanie wdrożone. Mam przeczucie, że najskuteczniejszym celem jest gdzieś pomiędzy dwiema opcjami.

Jaki jest zatem cel przeglądu kodu?

code-reviews theory SoylentGray
źródło
16
Powiązane (na temat przepełnienia stosu): Cel recenzji kodu
yannis
16
- Skąd miałbyś wiedzieć, czy napisałeś czytelny i łatwy w utrzymaniu kod? - Twój kolega mówi ci po przejrzeniu kodu. Uzasadnienie: Nie możesz tego ustalić sam, ponieważ wiesz więcej jako autor niż sam kod mówi. Komputer nie może ci powiedzieć z tych samych powodów, z których nie może stwierdzić, czy obraz jest sztuką, czy nie. Dlatego potrzebujesz innego człowieka - zdolnego do utrzymania oprogramowania - aby spojrzeć na to, co napisałeś i wyrazić swoją opinię. Formalna nazwa tego procesu to „Wzajemna ocena” . ”
komar
3
„Do czego służy przegląd kodu?” aby uniemożliwić programistom pisanie kodu terribad i kierować ich we właściwym kierunku.
zzzzBov
7
Wygląda na to, że Code Review może mieć pośrednią odpowiedź na to pytanie. Po prostu przejrzyj tam pytania i odpowiedzi, cel przeglądu kodu stanie się dość widoczny :)
Mathieu Guindon
3
Zastanawiam się, ile razy programiści odkryli błędy we własnym kodzie tylko poprzez prosty proces wyjaśniania swojego kodu podczas recenzji?
topór nieaktywny

Odpowiedzi:

75

Istnieje wiele powodów, dla których warto przeprowadzić przegląd kodu:

  • Edukacja innych programistów. Upewnij się, że wszyscy widzą modyfikację związaną z poprawką lub ulepszeniem defektu, aby mogli zrozumieć resztę oprogramowania. Jest to szczególnie przydatne, gdy ludzie pracują nad komponentami, które muszą być zintegrowane, lub w złożonych systemach, w których jedna osoba może chodzić przez dłuższy czas, nie patrząc na niektóre moduły.
  • Znalezienie wad lub możliwości poprawy. Zarówno kod dostarczalny, jak i kod testowy oraz dane mogą zostać zbadane w celu wykrycia słabych punktów. Zapewnia to, że kod testowy jest solidny i poprawny oraz że projekt i implementacja są spójne w całej aplikacji. Jeśli trzeba wprowadzić dodatkowe zmiany, zbliża się szansa bliżej punktu wejścia.

Istnieje kilka przypadków biznesowych dotyczących przeprowadzania recenzji:

  • Znalezienie wad lub problemów, które należałoby przerobić bliżej ich wstrzyknięcia. To jest tańsze.
  • Wspólne zrozumienie systemu i szkolenie przekrojowe. Mniej czasu na przyspieszenie przez programistę wprowadzenia zmian.
  • Identyfikacja możliwych ulepszeń systemu.
  • Otwarcie wdrożenia w celu zapewnienia, że ​​testerzy zapewniają odpowiedni zasięg. Przekształcanie czarnej skrzynki w szarą lub białą z perspektywy testowania.

Jeśli szukasz kompleksowej dyskusji na temat korzyści i strategii wdrażania w recenzjach, polecam sprawdzenie recenzji w oprogramowaniu: praktyczny przewodnik Karla Wiegera .

Thomas Owens
źródło
7
+1, myślę, że dobrze zauważyłeś główne punkty, z prawidłowymi priorytetami. Utrzymanie spójności projektu w kontaktach ze współpracownikami, którzy stale znajdują bardzo kreatywne rozwiązania „WTF”, można osiągnąć jedynie poprzez regularne przeglądy kodu.
Doc Brown,
Przeglądamy kod w naszym kodzie JavaScript, głównie po to, aby programiści przestrzegali określonych standardów, używali ustalonego wzorca podczas projektowania modułów, korzystali z dostarczonych komponentów i nie zaczęli kodować ninja (celowo lub nie) wokół problemów, które już mamy rozwiązania dla. Świetnie też zauważają, że ktoś przypadkowo przesadza z thiskontekstem, nie używając go .hasOwnPropertyw miejscach, w których powinien być itp., Itd. - A więc, głównie ze względu na standardy. W języku zarządzanym, takim jak C #, masz oczywiście mniej powodów niż języki dynamiczne.
Nie,
1
Jak dotąd twoja odpowiedź odnosi się tylko do ulepszeń poprawności kodu. Nie rozwiązuje problemu czytelności / konserwacji kodu, który rzadko może być dokładnie określony ilościowo przez programistę.
ArTs
51

Recenzje kodu to narzędzie do transferu wiedzy .

  • Gdy programiści dokonują wzajemnego przeglądu kodu, zapoznają się ze wszystkimi obszarami systemu. Zmniejsza to współczynnik magistrali projektu i sprawia, że ​​programiści są bardziej wydajni, gdy muszą wykonywać konserwację części systemu, której nie napisali.

  • Gdy młodszy programista sprawdza kod seniora, młodszy programista może wybierać sztuczki, które w innym przypadku byłyby możliwe tylko dzięki doświadczeniu. Może to również działać jako korekta przed zbyt skomplikowanym kodem.

    Dokładny przegląd kodu będzie wymagał częstych kontroli różnych dokumentów. To świetny sposób na naukę języka lub interfejsu API.

  • Kiedy starszy programista sprawdza kod młodszego, jest to okazja do rozwiązania problemów, zanim przełożą się one na zadłużenie techniczne. Przegląd kodu może być dobrym rozwiązaniem dla mentorów młodszych programistów.

Recenzje kodu nie dotyczą:

  • … Znajdowanie błędów. Po to są testy. Nadal często zdarza się, że przegląd kodu znajduje jakiś problem.

  • … Dręczące kwestie związane ze stylem - zadowalaj się jednym stylem i użyj automatycznych formaterów, aby go wymusić. Ale jest wiele rzeczy, których automatyczne narzędzie nie może sprawdzić. Recenzje kodu są dobrym miejscem do upewnienia się, że kod jest wystarczająco udokumentowany lub samodokumentujący.

amon
źródło
2
twój ostatni punkt na temat zagadnień związanych ze stylem gry, z którym nie do końca się zgadzam - mieliśmy właśnie wstrząsające doświadczenie w recenzowaniu kodu młodszego programisty i najbardziej rażąca skarga dotyczyła stylu, ale nie tego rodzaju problemów ze stylem, które łatwo programowo egzekwowane .... waaaaaaay zbyt wiele jeśli oświadczenia dla edgecases itp .; problemy, które tak, w niektórych przypadkach można było znaleźć na komputerze, ale większość z nich nie była ogólnie rzecz biorąc warta znalezienia na podstawie skryptu. Przeczytanie zajmuje nam 30 sekund, kolejne 30, aby wyjaśnić twórcy i mam nadzieję, że naprawię ten problem. Wciąż w szoku: /
pacyfista
7
@pacifist To nie jest styl, który opisuje odpowiadający. Styl dotyczy lokalizacji nawiasów klamrowych, wcięć itp. Jeśli twój młodszy programista używa zbyt wielu instrukcji, masz zupełnie inny problem niż styl; ogólną cechą kodowania STYLE jest to, że nie wpływa to na wydajność. I myślę, że znaczna liczba instrukcji if wpłynie na wydajność.
Pimgd,
12
Podczas recenzji często zauważam rzeczy, w których myślę, że „to wygląda jak błąd”, a następnie piszę konkretny przypadek testowy, aby udowodnić, że to błąd. Jednym z wielu celów przeglądów kodu jest znajdowanie błędów. Dlatego uważam, że punkt widzenia „albo przegląd kodu, albo testy” jest trochę zbyt jednostronny.
Doc Brown,
2
Oprócz @DocBrown istnieją przypadki, których nie można łatwo przetestować - wyścigi danych, niektóre typy zakleszczeń, blokady na żywo, niezdefiniowane zachowania / wartości (głównie w C / C ++, ale kolejność elementów w tabelach skrótów również w niezdefiniowanych) lub pora zasobów (otwieranie pliku w pętli może być złym pomysłem nawet w przypadku GC). Niektóre z tych rzeczy można wykryć za pomocą wystarczająco inteligentnej analizy statycznej kompilatora .
Maciej Piechotka,
2
@ pacifist Twój konkretny przykład zostałby całkowicie zniszczony podczas przeglądu kodu. Byłby również czerwoną flagą dla każdego analizatora kodu statycznego (złożoność cykliczna 17!). Przegląd kodu szybko zidentyfikowałby tę funkcję jako problem w stylu semantycznym (a nawet algorytmie!). Jednak. Ten rodzaj problemu to nie tylko kwestia „stylu”. Jeśli potraktujesz go w ten sposób, wkrótce będziesz mieć naprawdę paskudny kod w swoim repozytorium; w końcu to tylko „styl”.
Pimgd
12

Najcenniejszą rzeczą, którą osobiście otrzymuję z przeglądu kodu, jest pewność, że kod jest zrozumiały dla innej osoby. Czy zmienne są wyraźnie nazwane? Czy cel każdego fragmentu kodu jest dość oczywisty? Czy coś niejednoznacznego zostało wyjaśnione w komentarzu? Czy przypadki na krawędziach i prawidłowe wartości parametrów są opisane w komentarzach i sprawdzone w kodzie?


źródło
2
wydaje się, że nie oferuje to nic istotnego w porównaniu z poprzednimi 4 odpowiedziami
komnata
2
@gnat: Inne odpowiedzi dotyczą transferu wiedzy i wykrywania błędów. Są to ważne, ale przegląd kodu ma coś więcej.
1
o ile mogę powiedzieć, w tej odpowiedzi rozsądnie poruszono więcej kwestii: „Jeśli szukasz kompleksowej dyskusji na temat korzyści i strategii wdrażania dla recenzji, polecam sprawdzenie recenzji oprogramowania: Praktyczny przewodnik Karla Wiegera ”. Ta odpowiedź obejmuje również: „poprawkę przeciwko zbyt skomplikowanemu kodowi”
gnat
2
@gnat Podkreśla inny aspekt kwestii poruszonych w innych odpowiedziach. Czy byłeś kiedyś zaskoczony własnym kodem, który napisałeś sześć miesięcy temu? Przegląd kodu pozwala przyspieszyć ten proces. Jeśli twój współpracownik jest zdziwiony twoim kodem, możesz go wyjaśnić, dopóki problem jest wciąż świeży w twojej pamięci.
200_success
1
@ 200_sukces może. Ale ten punkt, jeśli rzeczywiście istnieje, wygląda naprawdę źle. Nawet twój komentarz udaje się przekazać lepiej niż ta „odpowiedź”. Nie wspominając już o tym, że po prostu powtarza to, co wskazano we wcześniejszym komentarzu, który odnosi się do kanonicznej odpowiedzi wyjaśniającej to w powiązanym pytaniu
gnat
7

Chciałbym dodać dwa obszary nieobjęte innymi świetnymi odpowiedziami:

Jednym z ważnych powodów recenzji kodu jest efekt Hawthorne'a, który w naszym przypadku przekłada się na: Jeśli wiesz, że ktoś będzie później patrzył na twój kod, to znacznie bardziej prawdopodobne jest, że lepiej go napiszesz.

Innym ważnym powodem są bezpieczniejsze praktyki programistyczne. Wystarczy spojrzeć na błąd goto Apple'a (przypadkowa zduplikowana linia kodu) lub błąd Heartbleed (podstawowy błąd w sprawdzaniu poprawności danych wejściowych), aby zrozumieć znaczenie prawidłowych recenzji kodu w bezpiecznym cyklu rozwojowym.

Chris Knight
źródło