Co robisz, gdy przegląd kodu jest po prostu zbyt trudny?

144

OK, więc przegląd kodu jest dość rutynowy. Ale czasami zdarzają się zmiany, które zasadniczo wpływają na istniejący złożony, delikatny kod. W tej sytuacji czas potrzebny do zweryfikowania bezpieczeństwa zmian, braku regresji itp. Jest nadmierny. Być może nawet przekraczając czas potrzebny na sam rozwój.

Co robić w tej sytuacji? Scalić i mieć nadzieję, że nic nie wymknie się? (Nie popieram tego!) Czy najlepiej, jak potrafisz, i staraj się tylko dostrzec jakiekolwiek oczywiste wady (być może jest to najbardziej przegląd kodu, do którego i tak należy dążyć?) Scalić i przetestować wyjątkowo dokładnie jako lepszą alternatywę niż przegląd kodu w ogóle?

Nie jest to konkretnie pytanie, czy testowanie należy wykonać w ramach przeglądu kodu. To pytanie zadaje najlepsze dostępne opcje w opisanej sytuacji, szczególnie w przypadku naglącego terminu, braku kompleksowego zestawu testów jednostkowych lub testów jednostkowych nie wykonalnych dla zmienionego fragmentu kodu.

EDYCJA: Mam wrażenie, że do tej pory kilka odpowiedzi / komentarzy dotyczyło mojego wyrażenia „szeroko pojęty wpływ” i być może uznałem, że zmiana dotyczy dużej liczby wierszy kodu. Rozumiem, że to interpretacja, ale tak naprawdę nie było to moim zamiarem. Przez „szeroki wpływ” rozumiem na przykład, że potencjał regresji jest wysoki ze względu na wzajemne powiązania bazy kodowej lub zakres efektów domina, niekoniecznie dlatego, że sama zmiana jest duża. Na przykład programista może znaleźć sposób na naprawienie błędu za pomocą jednej linii, wywołując istniejącą procedurę wysokiego poziomu, która kaskaduje połączenia z wieloma procedurami niższego poziomu. Testowanie i weryfikowanie, czy poprawka zadziałała, jest łatwe. Ręczna weryfikacja (poprzez przegląd kodu) wpływu wszystkich efektów domina jest znacznie trudniejsza.

Brad Thomas
źródło
91
Co z uruchomieniem zestawu testów, aby upewnić się, że niczego nie zepsułeś?
Vincent Savard
130
what if there is no pre-existing test suite?- Co powiesz na napisanie jednego?
Robert Harvey
27
Pakiet testowy zdecydowanie by pomógł. Ale wzajemna ocena i testy uzupełniają się. Myślę, że nie jest dobrym pomysłem zamiana jednego na drugi.
Christophe
8
@MasonWheeler: Prawdopodobnie rozmowa po raz kolejny, a ty odnosisz się do TDD konkretnie w tym artykule, przy założeniu, że nie sądzę, by jakikolwiek szanujący się TDD kiedykolwiek, ale zrobiłem to w obie strony, i uważam, że korzyści z testów jednostkowych są oczywiste.
Robert Harvey
21
Merge and hope nothing slips through?To bardzo zły pomysł.
Maszt

Odpowiedzi:

306

Założenie tego pytania jest, szczerze mówiąc, zdumiewające. Przypuszczamy, że nastąpiła duża zmiana w kruchym, złożonym kodzie i że po prostu nie ma wystarczająco dużo czasu na jego poprawne sprawdzenie . To ostatni kod, który powinieneś poświęcać mniej czasu na recenzowanie! To pytanie wskazuje, że masz problemy strukturalne nie tylko w samym kodzie, ale także w metodyce zarządzania zmianami.

Jak więc poradzić sobie z tą sytuacją? Zacznij od nie wchodzenia w to przede wszystkim:

  • Zidentyfikuj źródła złożoności i zastosuj ostrożne, dokładnie sprawdzone, poprawne refaktoryzacje, aby zwiększyć poziom abstrakcji. Kod powinien być zrozumiały dla nowego pracownika po studiach, który wie coś o Twojej domenie biznesowej.

  • Zidentyfikuj źródła niestabilności; może to być przejrzenie samego kodu, zbadanie historii poprawek błędów w kodzie i tak dalej. Określ, które podsystemy są delikatne i zwiększ ich niezawodność . Dodaj logikę debugowania. Dodaj asercje. Utwórz powolną, ale oczywiście poprawną implementację tego samego algorytmu, aw kompilacji debugowania uruchom obie i sprawdź, czy się zgadzają. W kompilacji debugowania sprawiaj, że rzadsze sytuacje występują częściej. (Na przykład stwórz alokator pamięci, który zawsze przenosi blok podczas realokacji lub zawsze alokuje blok na końcu strony itp.) Uczyń kod odpornym na zmiany w jego kontekście. Teraz nie masz już delikatnego kodu; teraz masz kod, który wykrywa błędy, a nie powoduje błędów.

  • Napisz pakiet automatycznych testów. Oczywiście.

  • Nie wprowadzaj szerokich zmian. Wprowadź serię drobnych, ukierunkowanych zmian, z których każda może być poprawna.

Ale zasadniczo twój scenariusz brzmi: „wkopaliśmy się w dziurę technicznego długu i każda złożona, niezbadana zmiana wciąga nas głębiej; co powinniśmy zrobić?”. Co robisz, gdy znajdziesz się w tej dziurze? Przestań kopać . Jeśli masz tak dużo długów, że nie jesteś w stanie wykonać podstawowych zadań, takich jak wzajemne sprawdzanie swojego kodu, musisz przestać zaciągać więcej długów i poświęcić czas na spłatę.

Eric Lippert
źródło
74
Z tego, co widziałem w branży, po „Stopping Digging” zwykle następuje szybkie zakończenie, a następnie znalezienie kogoś, kto chętnie skorzysta z łopaty. Ta odpowiedź powinna dodać zastrzeżenie, że peony z małp kodowych nie powinny tego robić bez przygotowania się na konsekwencje ...
Luke A. Leber
63
@Luke, jeśli kierownictwo lub starsi deweloperzy są tak nastawieni na orkę naprzód pomimo problemów, a nawet pomyślą o rozwiązaniu każdego, kto spróbuje przynieść trochę rozsądku do tej sytuacji (OK, rażące niesubordynacja na bok), firma jest w nieodwracalnym marszu śmierci. Zostaw im to.
Julia Hayward
14
@JuliaHayward Masz rację, ale sytuacja, którą opisuje Luke, jest powszechna, szczególnie w przypadku kodu, który już generuje przychody. Naprawdę od Ciebie zależy, czy warto nad tym pracować.
Owen
19
@ LukeA.Leber Masz rację. Pracowałem dla tych firm. Mogę ci powiedzieć, że marsz śmierci zajmie lata, ale z każdym miesiącem będzie coraz gorzej. „Code Monkeys” będzie coraz bardziej nieszczęśliwe każdego miesiąca, ale złym menadżerom zajmie lata, aby uświadomić sobie konsekwencje swoich działań… jeśli w ogóle.
JS.
10
@Matt: Założeniem pytania jest to, że ktoś dba wystarczająco o jakość kodu, aby wprowadzić formalny system przeglądu kodu oraz że osoba zadająca pytanie jest zaniepokojona wpływem dużych zmian na jakość kodu. Jeśli zamiast tego założymy, że nikt nie dba o jakość kodu, to pewnie, moja odpowiedź na temat sposobów zapewnienia jakości kodu nie ma zastosowania, ale nie o to pytano!
Eric Lippert,
96

Jednym z głównych celów przeglądu kodu jest poprawa jakości i dostarczenie niezawodnego kodu . Solidny, ponieważ 4 oczy zwykle dostrzegają więcej problemów niż 2. A recenzent, który nie napisał dodatkowego kodu, jest bardziej podatny na (potencjalnie błędne) założenia.

Unikanie recenzji wzajemnych przyczyniłoby się w twoim przypadku tylko do zwiększenia kruchości kodu. Oczywiście wzmocnienie testów za pomocą solidnego i powtarzalnego zestawu testów może z pewnością poprawić jakość. Powinno to jednak uzupełniać wzajemną ocenę, a nie zastępować .

Uważam, że należy zrozumieć i opanować złożoność, a pełna wzajemna ocena jest okazją do podzielenia się wiedzą i osiągnięcia tego. Inwestycja, którą poczynisz, aby coraz więcej osób rozumiało siłę i słabość delikatnego kodu, z czasem pomoże go ulepszyć.

Cytat na zakończenie:

„Jeśli chcesz iść szybko, idź sam. Jeśli chcesz iść daleko, idź razem”

Christophe
źródło
5
Rzeczywiście, wyobraź sobie, że „złożona” została zastąpiona „długą” lub „źle zaprojektowaną” lub „źle udokumentowaną” lub jakąkolwiek inną negatywną cechą, którą powiedzielibyśmy: „To nie jest dobry powód do przeglądu - naprawmy te problemy, aby można je było naprawić! „ i to nie jest inaczej.
corsiKa
11
Dodam również, że jeśli kod nie może być teraz sprawdzony, nie można go utrzymać za 6 miesięcy .....
corsiKa
3
@corsiKa Po co czekać 6 miesięcy, aż będzie nie do utrzymania?
krillgar
2
@krillgar It umm ... nie ... to tylko liczba, którą zerwałem z czubka głowy, aby reprezentować okres czasu, kiedy odłożyłeś kod i musisz go ponownie odebrać ... więc, tak ...
corsiKa
16
@krillgar: Piszę jakiś „nowy kod”, sprawdzam go, idę na lunch, a kiedy wrócę, mój „nowy kod” magicznie zmienił się w „starszy kod”. Jak to się stało? :)
Eric Lippert,
35

Witamy w świecie tworzenia oprogramowania starszego typu.

Masz setki tysięcy, milionów, dziesiątki milionów linii kodu.

Te wiersze kodu są cenne, ponieważ generują strumień przychodów, a ich zastąpienie jest nieosiągalne.

Twój model biznesowy opiera się na wykorzystaniu tej bazy kodu. Twój zespół jest mały, baza kodu jest duża. Dodanie funkcji jest wymagane, aby zachęcić użytkowników do zakupu nowej wersji kodu lub zadowolenia obecnych klientów.

W idealnym świecie twoja ogromna baza kodu jest testowana jednostkowo wazoo. Nie żyjesz w idealnym świecie.

W mniej idealnym świecie masz budżet na spłatę długu technicznego - podziel kod na części, które można przetestować w jednostce, wykonuj intensywne testy integracyjne i iteruj.

To jednak spłaca dług bez tworzenia nowych funkcji. Który nie pasuje do uzasadnienia biznesowego „czerpania zysków z istniejącego kodu, przy jednoczesnym modyfikowaniu go w celu wygenerowania zachęty do aktualizacji”.

Możesz wziąć ogromne fragmenty kodu i przepisać go przy użyciu bardziej nowoczesnych technik. Ale wszędzie, gdzie wchodzisz w interakcję z istniejącym kodem, ujawniasz możliwe punkty przerwania. Ten hack w systemie, którego się pozbyłeś, faktycznie skompensował dziwactwo w podsystemie, którego nie przepisałeś. Zawsze.

Co można zrobić, to postępować ostrożnie. Możesz znaleźć część kodu, którą faktycznie rozumiesz, a której zachowanie i interakcja z resztą systemu są dobrze zrozumiane. Możesz to zmodernizować, dodając testy jednostkowe i czyniąc jego zachowanie jeszcze wyraźniejszym.

Następnie znajdź części aplikacji, które głównie z nią współpracują, i atakuj je pojedynczo.

W ten sposób możesz ulepszyć podsystem, dodając funkcje, za które klienci są gotowi zapłacić.

Krótko mówiąc, jest to sztuka możliwego - dokonywania zmian bez niszczenia rzeczy, które stanowią uzasadnienie biznesowe.

Ale to nie twoje pytanie. Twoje pytanie brzmi: „Robię coś, co jest ogromne i które może popsuć różne rzeczy, i jak postępować zgodnie z najlepszymi praktykami?”

Robiąc coś wielkiego, prawdą jest, że jeśli chcesz to zrobić niezawodnie, w końcu poświęcasz więcej wysiłku na wykrywanie błędów i ich naprawianie niż na pisanie. Jest to ogólna zasada tworzenia oprogramowania: pisanie jest łatwe, a sprawne działanie jest trudne.

Prawdopodobnie masz nad głową powód biznesowy, w którym obiecałeś interesariuszowi, że ta ogromna zmiana nastąpi. I jest to „gotowe”, więc reagujesz na powiedzenie „nie, to nie jest zrobione, to po prostu wygląda lubię to".

Jeśli masz moc i budżet, faktycznie włóż wysiłek, aby zyskać pewność, że zmiana działa, lub po prostu odrzuć zmianę. Będzie to kwestia stopnia, a nie uprzejmości.

Jeśli nie masz tak dużej mocy, ale nadal ją masz, spróbuj nalegać, aby nowy system był testowalny jednostkowo . Jeśli przepiszesz jakiś podsystem, nalegaj, aby nowy podsystem składał się z małych części o ściśle określonych zachowaniach i testach jednostkowych wokół nich.

Potem jest najgorszy przypadek. Zagłębiasz się w długi. Pożyczasz wbrew przyszłości programu, mając więcej kruchego kodu i więcej błędów, aby uzyskać tę funkcję teraz i przekląć konsekwencje. Wykonujesz kontrolę jakości, aby znaleźć najgorsze problemy, a resztę zignorować. Jest to czasem właściwa odpowiedź z punktu widzenia biznesu, ponieważ jest teraz najtańsza. Zaciąganie długów w celu generowania zysków jest prawidłową strategią biznesową, szczególnie jeśli spłatę długu poprzez bankructwo (porzucenie kodu) jest na stole.

Dużym problemem jest to, że rzadko zachęty właścicieli firm są dostosowane do decydentów i programistów. Często występuje presja na „dostarczenie”, a generowanie prawie niewidocznego (dla przełożonych) długu technicznego jest świetną strategią krótko-, a czasem średnioterminową. Nawet jeśli przełożonych / interesariuszy będzie najlepiej służy nie tworząc cały ten dług.

Jak
źródło
3
Doświadczyłem większości z powyższych, tyle razy, że jest to przygnębiające. Mieszanka tandetnych praktyk programistycznych, przesunięcia słupków bramkowych i niesympatycznych terminów zarządzania oznacza, że ​​to, co wszyscy wiemy, powinno się zdarzyć, a to, co naprawdę się dzieje, to dwie bardzo różne rzeczy
Ben Hillier
4
To dobra odpowiedź, ponieważ podczas gdy wiele innych jest bardziej poprawnych technicznie - kompensuje to rzeczywisty świat i podczas gdy wszyscy chcemy żyć w świecie, w którym wszystko jest dobrze przetestowane, dobrze udokumentowane - nie robimy tego. Stary kod, dziwne implementacje, nieporozumienia, nieracjonalni interesariusze, zła pogoda ... życie rzuca na ciebie złe rzeczy i będziesz musiał sobie z tym poradzić.
Allan S. Hansen
25

Rozwiąż większe problemy, które powodują, że przegląd kodu jest zbyt trudny.

Te, które do tej pory zauważyłem:

  1. Brak zestawu testów jednostkowych
  2. Skomplikowane połączenia kodów, których można by uniknąć dzięki bardziej sensownej strukturze kodu i delegowaniu obowiązków związanych z kodowaniem
  3. Pozorny brak podstawowej architektury
Robert Harvey
źródło
15
  1. Możesz odesłać recenzję kodu z powrotem i poprosić programistę o podzielenie go na mniejsze, bardziej przyrostowe zestawy zmian i przesłać mniejszą recenzję kodu.

  2. Nadal możesz sprawdzać zapachy, wzorce i anty-wzorce kodu, standardy formatowania kodu, zasady SOLID itp., Bez konieczności przechodzenia przez każdy szczegół kodu.

  3. Nadal możesz przeprowadzać taktyczne kontrole kodu w celu prawidłowej weryfikacji danych wejściowych, blokowania / zarządzania wątkami, możliwych nieobsługiwanych wyjątków itp. Na poziomie szczegółowym, niekoniecznie rozumiejąc ogólną intencję całego zestawu zmian.

  4. Możesz przedstawić ocenę ogólnych obszarów ryzyka, o których wiesz, że może mieć wpływ kod, i poprosić programistę o potwierdzenie, że te obszary ryzyka zostały przetestowane jednostkowo (lub poprosić o napisanie automatycznych testów jednostkowych, a także o przesłanie ich do przeglądu ).

John Wu
źródło
14

W tej sytuacji czas potrzebny do zweryfikowania bezpieczeństwa zmian, braku regresji itp. Jest nadmierny.

Przeglądy kodu nie powinny mieć na celu przede wszystkim poprawności. Służą one poprawie czytelności kodu, łatwości konserwacji i zgodności ze standardami zespołu.

Znalezienie błędów poprawności podczas przeglądu kodu jest fajnym produktem ubocznym ich zrobienia, ale deweloper powinien upewnić się, że ich kod działa idealnie (w tym bez regresji) przed przesłaniem go do przeglądu .

Poprawność powinna być wbudowana od samego początku. Jeśli jeden programista nie jest w stanie go osiągnąć, poproś go, aby sparował program lub opracował plan z całym zespołem, ale nie traktuj go jako czegoś, co możesz dodać jako refleksję.

guillaume31
źródło
2
Zgadzam się, ale: Przeglądy kodu mają faktycznie zerowy cel, który jest nawet ważniejszy niż czytelność kodu, łatwość konserwacji itp. Służą do edukowania zespołu na temat jego standardów. Nawet jeśli nie dokonano żadnych edycji w wyniku przeglądu kodu, nadal spełniałyby 75% swojego celu, ponieważ recenzja wyszkoliłaby autora kodu, aby nie popełnił tego samego rodzaju błędów ponownie, wielokrotnie, przez cały długi okres użytkowania ten projekt i następny ...
Jonathan Hartley
1
Z pewnością może również odgrywać tę rolę, ale uważam, że programowanie w parach jest bardziej wydajne niż CR do wdrażania i wczesnej i średnioterminowej edukacji nowych członków zespołu. Pomyśl o trenerze, który siedzi obok ciebie przez cały czas ćwiczenia, a nauczycielem, który dokonuje oceny tylko po fakcie. Z mojego punktu widzenia poprawianie „ukończonej” pracy przez kogoś jest bardziej frustrujące i mniej edukacyjne niż praca wykonywana wspólnie z kimś.
guillaume31,
2
@JathanathanHartley: W takim przypadku (minus pierwszy) powód recenzji kodu sprawia, że ​​programiści piszą kod, którego nie wstydzą się pokazywać komuś innemu w recenzji kodu :-)
gnasher729,
Zgadzam się absolutnie z obu guillaume31 i gnasher729 powyżej.
Jonathan Hartley,
11

Jeśli uważasz, że przegląd kodu jest zbyt trudny, ponieważ zmienił kruchy kod, którego prawie nie można zmienić bez jego zepsucia, masz problem. Ale problem nie dotyczy przeglądu kodu. Problem nie dotyczy także testów jednostkowych, ponieważ łamliwego kodu nie można testować jednostkowo! Jeśli twój kod byłby testowalny jednostkowo, zostałby podzielony na małe, niezależne jednostki, z których każda może być przetestowana, i które działają dobrze razem, i dokładnie tego nie masz!

Masz więc mnóstwo kodów śmieci (aka „dług techniczny”). Najgorsze, co możesz zrobić, to zacząć naprawiać stertę kodu śmieci i nie kończyć pracy, ponieważ w ten sposób otrzymasz jeszcze większą stertę kodu śmieci. Pierwszą rzeczą jest, aby skłonić kierownictwo do zakupu w celu naprawy i dokończenia pracy. Albo ty nie. W takim przypadku po prostu go nie dotykasz.

Kiedy to naprawisz, wyodrębnisz jedną jednostkę z kodu, przekształcisz ją w coś, co ma dobrze zdefiniowane i dobrze udokumentowane zachowanie, napisze testy jednostkowe dla tej jednostki, przejrzy kod i modli się, aby nic się nie zepsuło. A potem robisz to samo z następną jednostką i tak dalej.

Ten trudny kawałek pojawia się, gdy napotkasz błędy. Twój kod szczurów będzie robił złe rzeczy w niektórych przypadkach, ponieważ są one tak kruche i skomplikowane, że coś pójdzie nie tak. Gdy wyodrębnisz jednostki, pozostały kod stanie się wyraźniejszy. (Miałem przypadek, w którym po pewnym refaktoryzacji funkcja zaczęła się od „if (condition1 && condition2 && condition3) crash ();”, co było dokładnie zachowaniem przed refaktoryzacją, tylko jaśniej. Następnie usunąłem ten wiersz :-) Zobaczysz dziwne i niepożądane zachowanie wyraźnie, więc możesz to naprawić. Z drugiej strony to tutaj musisz zmienić zachowanie istniejącego kodu, więc należy to zrobić ostrożnie).

gnasher729
źródło
3
Trudność polega na wytłumaczeniu firmie, że: „Tak, wprowadzimy kilka błędów, ale naprawimy je i naprawimy szybko. Trochę cierpliwości zapewni ci teraz nowe funkcje i poprawki błędów w przyszłości”.
RubberDuck,
3

Niestety, w momencie przeglądu kodu niewiele można z tym zrobić, poza kolejną filiżanką kawy. Rzeczywistym rozwiązaniem tego problemu jest rozwiązanie problemu długu technicznego, który narosłeś: kruche wzornictwo, brak testów. Mamy nadzieję, że masz przynajmniej funkcjonalną kontrolę jakości. Jeśli tego nie masz, zawsze modlisz się o niektóre kości kurczaka.

JimmyJames
źródło
3

Jeśli nie jesteś zadowolony z dostarczenia błędnego / niedziałającego oprogramowania i napraw go później, wówczas wysiłek V&V MUSI być dłuższy niż wysiłek rozwojowy!

Jeśli istniejący kod jest kruchy, pierwsze pytanie brzmi: „czy powinieneś go zmienić?” Kierownictwo musi zadzwonić, czy koszt / ryzyko przeprojektowania i ponownego wdrożenia tego kodu jest większy niż koszt / ryzyko ustalenia niestabilnego stosu śmieci. Jeśli jest to jednorazowe, łatwiej jest go po prostu załatać. Jeśli w przyszłości będą prawdopodobnie potrzebne dalsze zmiany, lepszym rozwiązaniem może być podjęcie trafienia teraz, aby uniknąć większego bólu w przyszłości. Musisz porozmawiać o tym z kierownictwem, ponieważ udzielanie menedżerom dobrych informacji jest częścią pracy. Muszą podjąć tę decyzję, ponieważ jest to decyzja strategiczna, która przekracza poziom odpowiedzialności.

Graham
źródło
1

Z mojego doświadczenia zdecydowanie zalecam, abyś objął swój kod dużą ilością testów, zarówno jednostkowych, jak i integracyjnych, PRZED wprowadzeniem jakichkolwiek zmian w danym systemie. Ważne jest, aby pamiętać, że obecnie istnieje bardzo dobra liczba narzędzi do tego celu, bez względu na język, w którym się rozwijasz.

Ponadto istnieje narzędzie wszystkich narzędzi do tworzenia testów integracyjnych. Tak, mówię o kontenerach, a zwłaszcza o Docker i Docker Compose . Pięknie zapewnia nam to sposób na szybkie skonfigurowanie złożonego środowiska aplikacji z infrastrukturą (baza danych, mongodb, serwery kolejek itp.) I aplikacjami.

Narzędzia są dostępne, używaj ich! :)

Cristianoms
źródło
1

Nie wiem, dlaczego jeszcze nie wspomniano, ale te 2 są najważniejszymi utworami:

  • Dzielisz listę zmian na wiele mniejszych list zmian, które następnie przeglądasz jedna po drugiej. *
  • Jeśli przegląd listy zmian nie skutkuje decyzją, że lista zmian wydaje się być dobra, oczywiście odrzucasz zmianę.

* Przykład: Zamieniasz bibliotekę A na bibliotekę B. Jedna lista zmian wprowadza bibliotekę B, różne różne listy zmian zastępują użycie A literą B kawałek po kawałku (np. Jedna lista zmian na moduł), a ostatnia lista zmian usuwa bibliotekę A.

Piotr
źródło
1

Czy najlepiej, jak potrafisz, i próbuj dostrzec tylko oczywiste wady (być może jest to i tak najbardziej ocena kodu powinna być celem)?

Nie lekceważ potencjalnej wartości poprawek kodu. Mogą być dobrzy w wykrywaniu błędów:

  • Znajdź błędy, które byłyby trudne do wykrycia podczas testowania
  • Znajdź błędy, które trudno byłoby zidentyfikować / naprawić podczas testowania

Są również przydatne z innych powodów:

  • Pomagaj w szkoleniu członków zespołu
  • Pomóż upewnić się, że kod spełnia inne wskaźniki jakości, np. Pomóż upewnić się, że jest zrozumiały i łatwy w utrzymaniu, a nie tylko wolny od błędów

Co robić w tej sytuacji?

W najlepszym / idealnym przypadku, przekazanie kontroli kodu nie oznacza tylko „braku oczywistych błędów”: oznacza „oczywiście brak błędów” (choć oczywiście chcesz to również przetestować).

Jeśli nie możesz zweryfikować nowej bazy kodu za pomocą inspekcji kodu, konieczne będzie przeprowadzenie bardziej szczegółowych testów „czarnej skrzynki”. Możesz być przyzwyczajony do cyklu programowania, w którym kod jest wprowadzany do produkcji po przejściu kontroli, ale jeśli nie można „przejść kontroli”, to nie można „wprowadzić go do produkcji” i potrzebuje on dłuższego cyklu: np. Testy integracyjne , testy systemowe, testy alfa, testy akceptacyjne, testy beta itp.

brak dostępnego kompleksowego zestawu testów jednostkowych lub testy jednostkowe nie są wykonalne dla zmienionego fragmentowanego kodu

Co z testami integracyjnymi, systemowymi i akceptacyjnymi?

W każdym razie powinieneś prawdopodobnie powiedzieć kierownikowi projektu i menedżerowi produktu, że kod jest prawie na pewno błędny, z nieznaną liczbą błędów; i że „dostaną to, co sprawdzają” zamiast po prostu „tego, czego oczekują” - tj. że jakość kodu nie jest lepsza niż ich testowanie (ponieważ jakość kodu nie była i nie może być zagwarantowana przez kontrolę kodu) .

Powinni ewentualnie przekazać tę wiadomość klientowi lub użytkownikom, dlatego przeprowadzają testy wersji beta (jeśli chcą zostać nowymi użytkownikami) lub używają starszej wersji, dopóki nowa wersja nie wyjdzie poza wersję beta (jeśli nie jest).

ChrisW
źródło
0

Dużo kodu jest pisane i scalane bez odpowiedniej recenzji kodu. To może działać. Istnieje powód, dla którego nazywa się to kodowym zapachem, a nie „złamanym kodem” lub coś w tym rodzaju. Brak recenzji kodu jest znakiem ostrzegawczym, a nie zwiastunem zagłady.

Rozwiązaniem tego problemu jest to, że nie ma jednego rozwiązania, które pasowałoby do wszystkich skrzynek, które możemy spakować w odpowiedzi typu StackExchange. Istnieje silna zgoda społeczności programistów, że przegląd kodu jest kluczową „najlepszą praktyką” i w tym przypadku jest pomijany. Twój rozwój nie obejmuje już wąskiego kanału „przestrzegania wszystkich najlepszych praktyk”. Musisz znaleźć własną drogę.

Czym jest „najlepsza praktyka”? Kiedy od razu do tego podchodzisz, jest to zestaw praktyk, które ludzie uważają, że ulepszają kod. Czy poprawiają kod? Na pewno nie! Internet jest zaśmiecony opowieściami firm, które postępowały zgodnie z „najlepszymi praktykami” i wpadły w pułapkę. Być może lepszym punktem widzenia „najlepszych praktyk” jest to, że są to rozwiązania typu „odpal i zapomnij” świata oprogramowania. Nie mogę nic wiedzieć o twojej firmie, twoim projekcie, zespole i być w stanie wymyślić „najlepsze praktyki” jako rzeczy, które ci pomogą. Są ogólną radą „nie szkodzić”.

Wyraźnie odstąpiłeś od tego planu. Na szczęście to rozpoznajesz. Dobra robota! Mówią, że wiedza to połowa sukcesu; jeśli tak, świadomość stanowi ponad połowę tego! Teraz potrzebne jest rozwiązanie. Z twojego opisu jasno wynika, że ​​środowisko biznesowe, w którym się znajdujesz, ewoluowało do tego stopnia, że ​​nudna rada „idź, dokonaj przeglądu kodu, to najlepsza praktyka” nie pozwoli go skrócić. W tym celu zalecam kluczową zasadę, której używam, jeśli chodzi o najlepsze praktyki oprogramowania:

Żadna najlepsza praktyka w zakresie tworzenia oprogramowania nie przewyższa potrzeb biznesowych.

Szczerze mówiąc, płacą wypłatę, a przetrwanie firmy jest zwykle o wiele ważniejsze niż jakość oprogramowania. Nie chcemy tego przyznać, ale doskonale napisane oprogramowanie jest bezużyteczne, jeśli jest uwięzione w ciele firmy umierającej z powodu wysiłków na rzecz utrzymania tego doskonale napisanego oprogramowania.

Więc gdzie idziesz? Podążaj śladem siły. Zauważyłeś, że z jakiegoś nieokreślonego powodu nie ma sensu poddawać się przeglądowi kodu dla jakiegoś zadania. Z mojego doświadczenia wynika, że ​​ten powód jest zawsze doczesny. Zawsze jest to albo „za mało czasu”, albo „za mało pieniędzy, aby pensje płynęły, gdy spędzasz czas”. To jest biznes; w porządku. Gdyby to było łatwe, wszyscy by to zrobili. Podążaj śladem siły w górę i znajdź kierownictwo, które jest w stanie pomóc ci zrozumieć, dlaczego przegląd kodu nie jest opcją. Język jest trudny i dość często dekret ścieka z wyższego kierownictwa i ulega zniekształceniu. Rozwiązanie twojego problemu może być ukryte w tym zniekształceniu.

Odpowiedź na to pytanie jest koniecznie konkretnym scenariuszem przypadku. Jest to podobne do próby przewidzenia, czy rzut monetą będzie głową czy reszką. Najlepsze praktyki mówią, aby obrócić go 100 razy, a oczekiwanie wyniesie około 50 głów i 50 ogonów, ale nie masz czasu, aby obrócić go 1 raz. Tutaj ważne są szczegóły twojej sytuacji. Czy wiesz, że moneta zazwyczaj wyląduje w tej samej orientacji, w jakiej została rzucona przez około 51% czasu? Czy poświęciłeś czas, aby zobaczyć, w którą stronę jest moneta, zanim rzucisz? To może coś zmienić.

Jednym z ogólnych rozwiązań, które mogą być dostępne, jest próba znalezienia sposobu na opracowanie procesu przeglądu kodu i uczynienie go bardzo niskim kosztem. Znaczna część kosztu procesu recenzji kodu polega na tym, że każdy jest w 100% poświęcony recenzji kodu podczas jego wykonywania. Tak musi być, ponieważ po zakończeniu przeglądu kodu kod jest błogosławiony. Być może możesz umieścić kod w innej gałęzi i dokonać przeglądu kodu równolegle z programowaniem na głównym pniu. A może możesz to nawet skonfigurować, aby oprogramowanie przeprowadzało testy za Ciebie. Być może działasz w środowisku biznesowym, w którym klienci mogą uruchamiać „nowy” kod równolegle ze starym i porównywać wyniki. Dzięki temu klienci stają się grupą urządzeń do tworzenia przypadków użycia.

Kluczem do tych wszystkich działających „maybes” jest to, że powinieneś starać się, aby twój kod łatwo rozpadł się na części. Być może będziesz w stanie „udowodnić” fragmenty kodu bez polegania na formalnej weryfikacji kodu poprzez użycie ich w mniej krytycznych projektach. Łatwiej jest to zrobić, jeśli zmiany są mniejsze, nawet jeśli ich suma jest zbyt duża, aby przejrzeć.

Ogólnie szukaj rozwiązań specyficznych dla Twojego projektu, firmy, zespołu. Ogólną odpowiedzią było „najlepsze praktyki”. Nie używasz ich, więc tym razem powinieneś poszukać bardziej niestandardowych rozwiązań tego problemu. To jest biznes. Gdyby wszystko szło zgodnie z oczekiwaniami, IPO byłyby o wiele łatwiej przypisywać wartości, prawda?

Jeśli zastąpienie recenzji kodu jest walką, pamiętaj, że nigdy nie było ani jednego fragmentu kodu, który sprawdziłby się w sprawdzaniu kodu. * Wszystko, co robi przegląd kodu, daje ci zaufanie do kodu i możliwość wprowadzenia poprawek zanim staną się problemem. Oba te cenne produkty przeglądu kodu można uzyskać w inny sposób. Przegląd kodu ma po prostu uznaną wartość, ponieważ jest w tym szczególnie dobry.

* Cóż, prawie: mikrojądro L4 otrzymało już przegląd kodu przez zautomatyzowany system proof, który dowodzi, że jego kod, jeśli skompilowany przez zgodny kompilator C ++, zrobi dokładnie to, co mówi dokumentacja.

Cort Ammon
źródło
2
Twoja odpowiedź sugeruje, że „automatyczny system sprawdzający” automatycznie sprawdzał kod źródłowy L4. W rzeczywistości dokonał przeglądu napisanego przez człowieka dowodu poprawności L4. Wykonanie dowodu zajęło lata. Niemniej jednak z tego przedsięwzięcia można się wiele nauczyć, jak pisać poprawny kod. (Żeby było jasne, nie jest to dowód na papierze i długopisu, ale dowód do odczytu maszynowego, który faktycznie „importuje” cały kod źródłowy i powody tego. Zobacz ssrg.nicta.com.au/publications/nictaabstracts/3783 .pdf )
Artelius
0

Jak podkreśla @EricLippert w swojej doskonałej odpowiedzi, tego rodzaju zmiana wymaga większej uwagi, a nie mniej . Jeśli zauważysz, że zmiana, nad którą pracujesz, stanie się taką zmianą, istnieje kilka strategii, które mogą pomóc:

  • Często zobowiązuj się do kontroli wersji. Recenzja może postępować na zasadzie zatwierdzania po zatwierdzeniu i może być bardziej zrozumiała, gdy masz mniejsze zatwierdzenia.
  • Pamiętaj o komentowaniu przyczyn każdej zmiany tak jasno, jak to możliwe.
  • Jeśli to w ogóle możliwe, użyj programowania par dla tego rodzaju zmian. Posiadanie trzech spojrzeń na problem zamiast dwóch może pomóc w uniknięciu problemów, które można normalnie pominąć, a posiadanie pary podczas pracy może pomóc poprawić wszelkie komentarze do kodu, który Twoim zdaniem był oczywisty, ale który okazuje się mniej oczywisty niż sądziłeś, co z kolei pomoże recenzentowi później. Pomoc w (a) ograniczeniu błędów podczas programowania i (b) ulepszeniu dokumentacji może faktycznie oznaczać, że spędza się na tym mniej roboczogodzin, pomimo zaangażowania większej liczby osób.
Jules
źródło
0

Więcej odpowiedzi dotyczy sposobu dotarcia do tego punktu. Wielu z nich podaje pewne sugestie, jak zaradzić tej sytuacji, ale chciałbym przekazać moją odpowiedź, aby udzielić krótkiej odpowiedzi.

Co zrobić, gdy recenzje kodu są „zbyt trudne?”

  1. Wróć do głównej gałęzi kodu linii
  2. Napisz testy funkcjonalności, którą zrefaktorowałeś (np. Testy funkcjonalne)
  3. Zalicz testy
  4. Scal testy z kodem „trudnym do przetestowania”
  5. Czy testy nadal przebiegają pomyślnie?

tak

Wy, programiści, byliście świetni! Koty wracają dla wszystkich!

(lub dla tych, którzy nie dorastali oglądając „ The Simpsons ” w amerykańskiej telewizji: jeśli testy przechodzą pomyślnie, pomiń próbowanie spojrzenia na różnice i poproś programistę, aby poprowadził Cię po trasie zmian)

Nie

Kontynuuj refaktoryzację i dodawanie pokrycia testowego, aż testy zakończą się pomyślnie.

Greg Burghardt
źródło
7
Co oznaczają koty z powrotem ?
JDługosz
@ JDługosz Jest teraz odniesieniem do Simpsonów .
Rhymoid
Nie rozumiem
JDługosz
Instruktor gimnastyki Lugash ma zwyczaj konfiskaty kotów i psów swoich uczniów, zwracając je tylko wtedy, gdy uczeń wykonał zadanie fizyczne. simpsons.wikia.com/wiki/Lugash
Mark McLaren
-1

Podobnie jak w przypadku mnożenia, przegląd kodu daje zero wyniku, gdy jest zastosowany do zera. W takim przypadku nie zwiększa wartości, podczas gdy w większości innych przypadków tak.

Kod, z którym musisz pracować, jest zbyt źle zaprojektowany, aby korzystać z procesu sprawdzania kodu podczas dalszego rozwoju. Użyj procesu sprawdzania kodu, aby go refaktoryzować lub ponownie opracować.

Być może kod nadal jest znośny, ale zadanie nie jest dobre. Jest zbyt szeroki i powinien być wykonywany w mniejszych krokach.

h22
źródło
2
@Downvoter, przegląd kodu nie zastępuje złego projektu, a próby jego zastosowania i tak zwykle powodują, że zmiany nigdy nie są zatwierdzane, ponieważ recenzent nie rozumie tych śmieciowych śmieci. Przepraszam, że zrujnowałem twoją wizję.
h22