Jak poradzić sobie z nieporozumieniem podczas przeglądu kodu dotyczącego mało prawdopodobnego przypadku na krawędzi?

Pracuję w startupie robotyki w zespole zajmującym się obsługą ścieżek i po przesłaniu żądania ściągnięcia mój kod jest sprawdzany.

Mój kolega z zespołu, który pracuje w zespole od ponad roku, skomentował mój kod, sugerując, że wykonuję o wiele więcej pracy, niż uważam za konieczne. Nie, nie jestem leniwym programistą. Uwielbiam elegancki kod, który ma dobre komentarze, nazwy zmiennych, wcięcia i poprawnie obsługuje przypadki. Ma jednak na myśli inny rodzaj organizacji, z którą się nie zgadzam.

Podam przykład:

Spędziłem dzień, pisząc przypadki testowe dotyczące zmiany algorytmu znajdowania przejścia, którego dokonałem. Zasugerował, że zajmę się niejasną sprawą, która jest bardzo mało prawdopodobna - w rzeczywistości nie jestem pewien, czy to możliwe. Utworzony przeze mnie kod działa już we wszystkich naszych oryginalnych przypadkach testowych i niektórych nowych, które znalazłem. Utworzony przeze mnie kod przechodzi już ponad 300 symulacji, które są uruchamiane co noc. Jednak poradzenie sobie z tą niejasną sprawą zajęłoby mi 13 godzin, które lepiej byłoby poświęcić na poprawę wydajności robota. Żeby było jasne, poprzedni algorytm, którego używaliśmy do tej pory, również nie zajmował się tym niejasnym przypadkiem i ani razu, w 40 000 wygenerowanych raportach, nigdy nie wystąpił. Jesteśmy startupem i musimy opracować produkt.

Nigdy wcześniej nie miałem recenzji kodu i nie jestem pewien, czy jestem zbyt kłótliwy; czy powinienem być cicho i robić to, co on mówi? Postanowiłem nie spuszczać głowy i po prostu dokonać zmiany, mimo że zdecydowanie nie zgadzam się z tym, że to był dobry użytek z czasu.

Szanuję mojego współpracownika i uznaję go za inteligentnego programistę. Po prostu się z nim nie zgadzam i nie wiem, jak poradzić sobie z nieporozumieniem podczas przeglądu kodu.

Wydaje mi się, że odpowiedź, którą wybrałem, spełnia te kryteria wyjaśniania, w jaki sposób młodszy programista może poradzić sobie z nieporozumieniem podczas przeglądu kodu.

niejasny przypadek, który jest niezwykle mało prawdopodobny - w rzeczywistości nie jestem pewien, czy to możliwe

Nieprzestrzeganie zachowań w kodzie może być bardzo ważne. Jeśli fragment kodu zostanie uruchomiony np. 50 razy na sekundę, szansa na milion pojawi się mniej więcej co 5,5 godziny działania. (W twoim przypadku szanse wydają się niższe).

Możesz porozmawiać o priorytetach ze swoim przełożonym (lub kimkolwiek, kto jest osobą starszą odpowiedzialną za jednostkę, w której pracujesz). Lepiej zrozumiesz, czy np. Praca nad wydajnością kodu lub kodowanie kuloodporne jest najwyższym priorytetem i jak nieprawdopodobne może być to narożnik. Twój recenzent może mieć również wypaczone pojęcie priorytetów. Po rozmowie z osobą odpowiedzialną łatwiej będzie ci (nie) zgodzić się z sugestiami recenzenta i będziesz mieć do czego się odwołać.

Zawsze warto mieć więcej niż jednego recenzenta. Jeśli Twój kod jest sprawdzany tylko przez jednego kolegę, poproś kogoś innego, kto zna ten kod lub bazę kodów ogólnie, o sprawdzenie. Druga opinia ponownie pomoże ci łatwiej (nie) zgodzić się z sugestiami recenzenta.

Posiadanie wielu powtarzających się komentarzy podczas kilku przeglądów kodu zwykle wskazuje na to, że większa rzecz nie jest wyraźnie komunikowana, a te same problemy pojawiają się wielokrotnie. Spróbuj znaleźć tę większą rzecz i przedyskutuj ją bezpośrednio z recenzentem. Zapytaj wystarczająco dlaczego pytania. Bardzo mi pomogło, kiedy zacząłem ćwiczyć recenzje kodu.

Mogę podać przykład sprawy narożnej, która nigdy nie mogłaby wystąpić, która spowodowała katastrofę.

Podczas opracowywania Ariane 4 wartości z bocznych przyspieszeniomierzy były skalowane, aby pasowały do 16-bitowej liczby całkowitej ze znakiem i ponieważ maksymalna możliwa moc akcelerometrów, po skalowaniu, nigdy nie może przekroczyć 32767, a minimum nigdy nie może spaść poniżej - 32768 „nie było potrzeby sprawdzania zasięgu”. Zasadniczo wszystkie dane wejściowe powinny być sprawdzane przed każdą konwersją, ale w tym przypadku byłoby to próbą uchwycenia niemożliwego przypadku narożnego.

Kilka lat później opracowywano Ariane 5 , a kod skalowania bocznych akcelerometrów został ponownie wykorzystany przy minimalnym testowaniu, ponieważ został „sprawdzony w użyciu”. Niestety, większa rakieta mogła oczekiwać większych przyspieszeń bocznych, więc akcelerometry zostały zaktualizowane i mogły wytwarzać większe 64-bitowe wartości zmiennoprzecinkowe .

Te większe wartości „zawinięte” w kod konwersji, nie pamiętają sprawdzania zakresu, a wyniki pierwszego uruchomienia w 1996 roku nie były dobre. Kosztowało to miliony firmy i spowodowało dużą przerwę w programie.

Chodzi mi o to, aby nie ignorować przypadków testowych, ponieważ nigdy się nie zdarzają, są bardzo mało prawdopodobne itp .: standardy, dla których kodowały, wymagały sprawdzenia zakresu wszystkich zewnętrznych wartości wejściowych. Gdyby to zostało przetestowane i załatwione, katastrofie można by zapobiec.

Zauważ, że w Ariane 4 nie był to błąd (ponieważ wszystko działało dobrze dla każdej możliwej wartości) - było to niezgodne ze standardami. Gdy kod został ponownie użyty w innym scenariuszu, zawiodł katastrofalnie, podczas gdy gdyby zakres wartości został przycięty, prawdopodobnie zawiodłby z wdziękiem, a istnienie przypadku testowego dla tego mogłoby spowodować przegląd wartości. Warto również zauważyć, że podczas gdy koderzy i testerzy zgłosili się do krytyki ze strony śledczych po wybuchu, odpowiedzialność za zarządzanie, kontrolę jakości i przywództwo spoczywała w większości.

Wyjaśnienie

Chociaż nie całe oprogramowanie ma kluczowe znaczenie dla bezpieczeństwa, ani nie jest tak spektakularne, gdy zawodzi, moim zamiarem było podkreślenie, że „niemożliwe” testy mogą nadal mieć wartość. To najbardziej dramatyczny przypadek, jaki znam, ale robotyka może również przynieść katastrofalne skutki.

Osobiście powiedziałbym, że gdy ktoś zwróci uwagę zespołowi testowemu na przypadek narożny, należy przeprowadzić test, aby to sprawdzić. Kierownik zespołu wdrażającego lub kierownik projektu może zdecydować, że nie będzie próbować zaradzić stwierdzonym awariom, ale powinien zdawać sobie sprawę z istnienia niedociągnięć. Alternatywnie, jeśli testowanie jest zbyt skomplikowane lub kosztowne, aby go wdrożyć, można poruszyć problem w każdym używanym module śledzącym i / lub rejestrze ryzyka, aby wyjaśnić, że jest to niesprawdzony przypadek - że może być konieczne rozwiązanie przed zmianą użytkowania lub zapobiec niewłaściwemu użyciu.

Ponieważ nie było to wcześniej obsługiwane, nie można tego zrobić. Ty lub twój kolega możecie zapytać swojego przełożonego, czy warto podjąć wysiłek, aby omówić tę sprawę.

Dzięki złożonym algorytmom bardzo trudno jest udowodnić, że pomyślałeś o każdym przypadku testowym, który pojawi się w prawdziwym świecie. Kiedy celowo zostawiasz uszkodzoną skrzynkę testową, ponieważ nie pojawi się ona w prawdziwym świecie, potencjalnie pozostawiasz uszkodzone inne skrzynki testowe, o których nawet jeszcze nie pomyślałeś.

Innym często występującym efektem jest obsługa dodatkowych przypadków testowych, algorytm z konieczności staje się bardziej ogólny, dzięki czemu można znaleźć sposoby uproszczenia go i uczynienia go bardziej niezawodnym dla wszystkich przypadków testowych. Oszczędza to czas na konserwację i rozwiązywanie problemów na drodze.

Ponadto istnieje wiele przypadków, w których na wolności występują błędy. Wynika to z faktu, że Twój algorytm może się nie zmienić, ale jego dane wejściowe mogą ulec zmianie w kolejnych latach, gdy nikt nie pamięta o tym jednym nieobsługiwanym przypadku użycia. Właśnie dlatego doświadczeni programiści zajmują się tego rodzaju rzeczami, gdy są świeżo zaprzątnięci. Wróci cię ugryźć później, jeśli tego nie zrobisz.

To nie jest pytanie techniczne, ale decyzja dotycząca strategii biznesowej. Zauważasz, że sugerowana poprawka dotyczy bardzo niejasnego przypadku, który prawie nigdy się nie wydarzy. To robi dużą różnicę, jeśli programujesz zabawkę lub programujesz powiedzmy sprzęt medyczny lub uzbrojony dron. Konsekwencje rzadkiej awarii będą bardzo różne.

Dokonując recenzji kodu, powinieneś zastosować podstawową wiedzę na temat priorytetów biznesowych przy podejmowaniu decyzji, ile zainwestować w obsługę rzadkich przypadków. Jeśli nie zgadzasz się ze swoim kolegą w kwestii interpretacji priorytetów firmy, możesz chcieć porozmawiać z kimś po stronie biznesowej rzeczy.

Przeglądy kodu nie dotyczą wyłącznie poprawności kodu. W rzeczywistości jest to znacznie poniżej listy korzyści, za dzieleniem się wiedzą i powolnym, ale stałym procesem w kierunku wypracowania konsensusu w stylu zespołu / projektu.

Kiedy doświadczasz, to, co liczy się jako „prawidłowe”, jest często dyskusyjne i każdy ma swój własny punkt widzenia na to, co to jest. Z mojego doświadczenia wynika, że ​​ograniczony czas i uwaga mogą powodować, że recenzje kodu są bardzo niespójne - ten sam problem może zostać wykryty lub nie w zależności od różnych programistów i w różnych momentach przez tego samego programistę. Recenzenci myślący „co poprawiłoby ten kod?” często sugerują zmiany, których nie dodaliby naturalnie do własnych wysiłków.

Jako doświadczony programista (ponad 15 lat jako programista) często jestem oceniany przez programistów z mniejszym doświadczeniem ode mnie. Czasami pytają mnie o zmiany, z którymi delikatnie się nie zgadzam lub które uważam za nieistotne. Nadal jednak wprowadzam te zmiany. Walczę o zmiany tylko wtedy, gdy uważam, że efekt końcowy spowodowałby słabość produktu, w którym koszt czasu jest bardzo wysoki lub gdy obiektywny punkt widzenia może być obiektywny (np. Prośba o zmianę wygrana) t pracować w języku, którego używamy, lub test porównawczy pokazuje, że deklarowana poprawa wydajności nie jest jednym).

Sugeruję więc ostrożnie dobierać swoje bitwy. Dwa dni kodowania przypadku testowego, który Twoim zdaniem nie jest konieczny, prawdopodobnie nie jest wart czasu / wysiłku na walkę. Jeśli korzystasz z narzędzia do sprawdzania, takiego jak żądania ściągania GitHub, możesz tam skomentować postrzegane koszty / korzyści, aby odnotować swój sprzeciw, ale zgodzić się na kontynuowanie pracy. To liczy się jako delikatne odsunięcie, więc recenzent wie, że przekraczają granicę, a co ważniejsze, dołącz swoje uzasadnienie, aby przypadki takie mogły zostać eskalowane, jeśli wpadną w impas. Chcesz uniknąć eskalacji pisemnych nieporozumień - nie chcesz spierać się na forum internetowym o różnice w pracy - więc pomocne może być omówienie problemu w pierwszej kolejności i zapisanie rzetelnego podsumowania wyników dyskusji,przyjazna dyskusja i tak zadecyduje dla was obojga).

Po wydarzeniu jest to dobry temat do przeglądu sprintu lub spotkań zespołu programistów itp. Przedstaw go tak neutralnie, jak to możliwe, np. „Podczas przeglądu kodu programista A zidentyfikował ten dodatkowy przypadek testowy, napisanie go zajęło dwa dni, zespół uważa, że ​​dodatkowe pokrycie było uzasadnione tym kosztem? ” - to podejście działa znacznie lepiej, jeśli faktycznie wykonujesz pracę, ponieważ pokazuje to w pozytywnym świetle; wykonałeś pracę i po prostu chcesz sondować zespół pod kątem niechęci do ryzyka w porównaniu z rozwojem funkcji.

Radziłbym przynajmniej twierdzić przeciwko niejasnej sprawie. W ten sposób nie tylko przyszli programiści zobaczą, że aktywnie zdecydowałeś się na tę sprawę, ale przy dobrej obsłudze awarii, która powinna już być na miejscu, przyniosłoby to również niespodzianki.

A następnie wykonaj test, który potwierdza tę awarię. W ten sposób zachowanie jest lepiej udokumentowane i pojawi się w testach jednostkowych.

Ta odpowiedź oczywiście zakłada, że ​​twoja ocena nawet bycia „wyjątkowo mało prawdopodobnym, jeśli to w ogóle możliwe” jest poprawna i nie możemy tego ocenić. Ale jeśli tak, a twój kolega się z tym zgadza, to wyraźne stwierdzenie przeciwko temu wydarzeniu powinno być satysfakcjonującym rozwiązaniem dla was obu.

Ponieważ wydaje się, że jesteś tam nowy, możesz zrobić tylko jedną rzecz - skonsultować się z kierownikiem zespołu (lub kierownikiem projektu). 13 godzin to decyzja biznesowa; dla niektórych firm / zespołów bardzo dużo; dla niektórych nic. To jeszcze nie twoja decyzja.

Jeśli trop mówi „zakryj tę skrzynkę”, w porządku; jeśli powie „nie, pieprzyć to”, w porządku - jego decyzja, jego odpowiedzialność.

Jeśli chodzi o recenzje kodu, zrelaksuj się. Zwrócenie zadania raz lub dwa razy jest całkowicie normalne.

Nie wydaje mi się, żeby jedna rzecz była skierowana w naturze, chociaż była to rodzaj wychowania w odpowiedzi na @SteveBarnes:

Jakie są konsekwencje niepowodzenia?

W niektórych polach błąd oznacza błąd na stronie internetowej. Niebieskie ekrany komputera i restarty.

Na innych polach to życie lub śmierć - samochód samojezdny zamyka się. Medyczny stymulator przestaje działać. Lub w odpowiedzi Steve'a: rzeczy wybuchają, powodując utratę milionów dolarów.

W tych ekstremach istnieje świat różnic.

To, czy 13 godzin na pokrycie „awarii” jest ostatecznie warte, nie powinno zależeć od ciebie. Powinno to zależeć od kierownictwa i właścicieli. Powinni poczuć większy obraz.

Powinieneś być w stanie zgadnąć, co będzie tego warte. Czy twój robot po prostu zwolni czy zatrzyma się? Zmniejszona wydajność? A może awaria robota spowoduje szkody finansowe? Utrata życia?

Odpowiedź na to pytanie powinna prowadzić do odpowiedzi „czy warto 13 godzin czasu firmowego”. Uwaga: Powiedziałem czas firm. Płacą rachunki i ostatecznie decydują, co jest tego warte. Kierownictwo powinno mieć ostatnie słowo w obu przypadkach.

Może porozmawiasz z osobą odpowiedzialną za nadanie priorytetu pracy? Podczas uruchamiania może być CTO lub właściciel produktu. Mógłby pomóc ustalić, czy ta dodatkowa praca jest wymagana i dlaczego. Możesz także przynieść swoje zmartwienia podczas codziennych pojedynków (jeśli je masz).

Jeśli nie ma wyraźnej odpowiedzialności (np. Właściciela produktu) za planowanie pracy, spróbuj porozmawiać z ludźmi wokół ciebie. Później może stać się problemem, że wszyscy popychają produkt w przeciwnym kierunku.

Najlepszy sposób na rozwiązanie sporu jest taki sam, niezależnie od tego, czy jesteś młodszym programistą, starszym programistą, a nawet dyrektorem generalnym.

Zachowuj się jak Columbo .

Jeśli nigdy nie oglądałeś żadnego Columbo, był to całkiem fantastyczny program. Columbo był tą bardzo skromną postacią - większość ludzi uważała, że ​​jest trochę szalony i nie warto się o niego martwić. Ale wyglądając na pokornego i prosząc ludzi o wyjaśnienia, był w stanie zdobyć swojego człowieka.

Myślę, że ma to również związek z metodą Sokratejską .

Zasadniczo chcesz zadawać pytania sobie i innym, aby upewnić się, że dokonujesz właściwych wyborów. Nie z pozycji „mam rację, mylisz się”, ale z pozycji uczciwego odkrycia. A przynajmniej najlepiej jak potrafisz.

W twoim przypadku masz tutaj dwa pomysły, ale mają one zasadniczo ten sam cel: ulepszenie kodu.

Masz wrażenie, że przeglądanie kodu w potencjalnie nieprawdopodobnym (niemożliwym?) Przypadku na rzecz rozwoju innych funkcji jest najlepszym sposobem na to.

Twój współpracownik ma wrażenie, że bardziej ostrożne podejście do narożnych skrzynek jest cenniejsze.

Co oni widzą, czego nie widzisz? Co widzisz, czego oni nie widzą? Jako młodszy programista masz naprawdę świetną pozycję, ponieważ naturalnie powinieneś zadawać pytania. W innej odpowiedzi ktoś wspomina, jak zaskakująco prawdopodobna jest sprawa narożna. Możesz więc zacząć od: „Pomóż mi zrozumieć - miałem wrażenie, że X, Y i Z - czego mi brakuje? Dlaczego widget zamieni się w ramkę? Miałem wrażenie, że będzie on sflaczały w burzliwych okolicznościach. swizzle stick faktycznie wykuwa pędzle ANZA? ”

Kiedy kwestionujesz swoje założenia i odkrycia, wykopiesz, odkryjesz uprzedzenia, a ostatecznie odkryjesz, jaki jest właściwy sposób działania.

Zacznij od założenia, że ​​wszyscy w Twoim zespole są całkowicie racjonalni i mają na względzie dobro zespołu i produktu, tak jak Ty. Jeśli robią coś, co nie ma sensu, musisz dowiedzieć się, czego nie wiesz, co robią lub co wiesz, że nie.

13 godzin to nic wielkiego, po prostu bym to zrobił. Pamiętaj, że zarabiasz za to. Po prostu określ to jako „bezpieczeństwo pracy”. Najlepiej też zachować dobrą karmę w zespole. Teraz, jeśli zajęło ci to tydzień lub dłużej, możesz zaangażować swojego menedżera i zapytać go, czy to najlepszy sposób wykorzystania twojego czasu, zwłaszcza jeśli się z tym nie zgadzasz.

Jednak brzmisz tak, jakbyś potrzebował dźwigni w swojej grupie. Oto jak uzyskać efekt dźwigni: prosić o wybaczenie, nie prosić o pozwolenie. Dodawaj rzeczy do programu według własnego uznania (w zakresie kursu, tj. Upewnij się, że całkowicie rozwiązuje problem, którego szef chce ...), i poinformuj kierownika lub kolegów po fakcie. Nie pytaj ich: „Czy mogę dodawać funkcję X”? Zamiast tego po prostu dodaj funkcje, które osobiście chcesz w programie. Jeśli denerwują się z powodu nowej funkcji lub się z nią nie zgadzają, możesz ją usunąć. Jeśli im się podoba, zatrzymaj to.

Ponadto, ilekroć proszą cię o zrobienie czegoś, przejdź „dodatkową milę” i dodaj wiele rzeczy, o których zapomnieli wspomnieć lub rzeczy, które działałyby lepiej niż to, co powiedzieli. Ale nie pytaj ich, czy „w porządku” pójść o krok dalej. Po prostu zrób to i niechlujnie powiedz im o tym po zakończeniu. Trenujesz ich ...

To, co się stanie, sprawi, że Twój menedżer będzie Cię określał jako „go-getter” i zacznie pokładać w tobie zaufanie, a twoi koledzy zaczną cię postrzegać jako lidera, od którego zaczynasz mieć program. A potem, gdy rzeczy, o których wspomniasz, zdarzą się w przyszłości, będziesz miał więcej do powiedzenia, ponieważ jesteś w zasadzie gwiazdą drużyny, a członkowie drużyny wycofają się, jeśli się z nimi nie zgodzisz.

Przegląd kodu służy kilku celom. Jedno, o czym oczywiście wiesz, to: „ Czy ten kod jest odpowiedni do celu? ” Innymi słowy, czy jest funkcjonalnie poprawny; czy jest odpowiednio przetestowany; czy nieoczywiste części zostały odpowiednio skomentowane; czy jest zgodny z konwencjami projektu?

Kolejną częścią przeglądu kodu jest dzielenie się wiedzą o systemie. Jest to okazja zarówno dla autora, jak i recenzenta, aby dowiedzieć się o zmienionym kodzie i jego interakcji z resztą systemu.

Trzecim aspektem jest to, że może zapewnić przegląd problemów, które istniały przed wprowadzeniem jakichkolwiek zmian. Dość często, gdy przeglądam czyjeś zmiany, zauważam coś, co przeoczyłem w poprzedniej iteracji (dość często coś własnego). Stwierdzenie typu „Oto okazja, aby uczynić to bardziej solidnym niż było”, nie jest krytyką i nie traktuj tego jako jednego!

Mój obecny zespół traktuje przegląd kodu nie tylko jako bramę lub przeszkodę, którą kod musi przejść bez szwanku przed zatwierdzeniem, ale przede wszystkim jako okazję do nieco uporządkowanej dyskusji na temat określonego obszaru funkcjonalności. To jedna z najbardziej produktywnych okazji do wymiany informacji. (I to dobry powód, aby podzielić się recenzją w zespole, a nie zawsze z tym samym recenzentem).

Jeśli postrzegasz recenzje kodu jako działanie konfrontacyjne, jest to samospełniająca się przepowiednia. Jeśli zamiast tego uważasz je za najbardziej współpracującą część pracy, będą one stymulować ciągłe doskonalenie Twojego produktu i sposobu współpracy. Pomaga, jeśli recenzja może jasno określić względne priorytety swoich sugestii - na przykład istnieje różnica między „Chciałbym tu przydatnego komentarza” a „To łamie się, jeśli xjest zawsze negatywne”.

Po złożeniu wielu ogólnych stwierdzeń powyżej, jak to się ma do twojej konkretnej sytuacji? Mam nadzieję, że teraz jest oczywiste, że moją radą jest odpowiedzieć na recenzję otwartymi pytaniami i wynegocjować, które podejście ma największą wartość. Na przykład w przypadku, gdy sugerowany jest dodatkowy test, to coś w stylu: „Tak, możemy to przetestować; szacuję, że wdrożenie zajmie <czas> . Czy uważasz, że korzyść jest tego warta? Czy jest coś jeszcze można zrobić, aby zagwarantować, że test nie jest konieczny? ”

Jedna rzecz, która mnie uderza, gdy czytam twoje pytanie: jeśli napisanie nowego przypadku testowego zajmuje dwa dni, to albo nowy test jest zupełnie innym scenariuszem niż istniejące testy (w takim przypadku prawdopodobnie ma dużo wartość) lub stwierdziłeś potrzebę lepszego ponownego wykorzystania kodu w pakiecie testowym.

Wreszcie, jako ogólny komentarz na temat wartości recenzji kodu (i jako zwięzłe podsumowanie tego, co powiedziałem powyżej), podoba mi się to oświadczenie, w Maintainers Don't Scale autorstwa Daniela Vettera :

Przynajmniej dla mnie przegląd nie polega tylko na zapewnieniu dobrej jakości kodu, ale także na rozpowszechnianiu wiedzy i poprawie zrozumienia. Na początku może jedna osoba, autor (i to nie jest dane), rozumie kod. Po dobrej recenzji powinny być co najmniej dwie osoby, które w pełni to rozumieją, w tym przypadki narożne.

Kod może ZAWSZE być lepszy.

Jeśli przeglądasz kod i nie widzisz niczego, co mogłoby być lepsze lub testu jednostkowego, który mógłby wykryć błąd, to nie jest to kod idealny, ale recenzent, który nie wykonuje swojej pracy. To, czy zdecydujesz się wspomnieć o usprawnieniu, jest osobistym wyborem. Ale prawie za każdym razem, gdy Twój zespół dokonuje przeglądu kodu, powinny być rzeczy, które ktoś zauważy, które mogą być lepsze lub wszyscy prawdopodobnie zmarnują swój czas.

To, czy zdecydujesz się na komentarze, czy nie, zależy od Twojego zespołu. Jeśli Twoje zmiany naprawią problem lub dodadzą wystarczającej wartości bez zmian, które Twój zespół akceptuje, to połącz je i zaloguj swoje komentarze do rejestru, aby ktoś mógł się później zająć. Jeśli zespół stwierdzi, że twoje zmiany powodują większe ryzyko lub złożoność niż wartość, musisz odpowiednio rozwiązać problemy.

Pamiętaj tylko, że każdy kod ma co najmniej jeszcze jeden przypadek brzegowy, który można przetestować i który może użyć co najmniej jeszcze jednego refaktoryzacji. Właśnie dlatego recenzje kodu najlepiej przeprowadzać jako grupę, w której wszyscy oglądają ten sam kod w tym samym czasie. Aby w końcu wszyscy mogli dojść do konsensusu co do tego, czy przeglądany kod jest akceptowalny (tak jak jest) i wnosi wystarczającą wartość do scalenia z bazą społeczności lub czy pewne rzeczy muszą zostać wykonane, zanim będzie wystarczająca wartość do scalenia .

Ponieważ zadajesz to pytanie, zakładam, że tak naprawdę nie robisz „recenzji kodu”, ale tworzysz żądanie ściągnięcia lub inny mechanizm przesyłania, aby inni mogli komentować w sposób niedeterministyczny. Teraz masz problem z zarządzaniem i definicję ukończenia. Sądzę, że twoje zarządzanie jest niezdecydowane i tak naprawdę nie rozumie procesu i celu recenzji kodu i prawdopodobnie nie ma „definicji ukończenia” (DOD). Ponieważ gdyby to zrobili, twój DOD odpowiedziałby wprost na to pytanie i nie musiałbyś tu przychodzić i pytać.

Jak to naprawić? Cóż, poproś swojego menedżera, aby dał ci DOD i powie ci, czy musisz zawsze wdrażać wszystkie komentarze. Jeśli dana osoba jest Twoim przełożonym, odpowiedź jest oczywista.

To pytanie nie dotyczy zalet programowania obronnego, niebezpieczeństw związanych z przypadkami narożnymi ani katastrofalnego ryzyka błędów w produktach fizycznych. W rzeczywistości w ogóle nie chodzi o inżynierię oprogramowania .

Tak naprawdę chodzi o to, jak młodszy praktykujący radzi sobie z instrukcją od starszego praktyka, gdy młodszy nie może się z tym zgodzić lub docenić.

Są dwie rzeczy, które musisz docenić będąc młodszym programistą. Po pierwsze, oznacza to, że chociaż możliwe jest , że masz rację, a on nie ma racji, jest mało prawdopodobne. Jeśli twój współpracownik sugeruje, że nie widzisz wartości, musisz poważnie zastanowić się nad możliwością, że po prostu nie masz wystarczającego doświadczenia, aby to zrozumieć. Nie rozumiem tego postu.

Drugą rzeczą, którą należy docenić, jest to, że twój starszy partner jest tak zwany, ponieważ ma większą odpowiedzialność. Jeśli junior złamie coś ważnego, nie będą mieli kłopotów, jeśli zastosują się do instrukcji. Gdyby jednak senior pozwolił im to zepsuć - nie podnosząc na przykład problemów z przeglądaniem kodu - słusznie mieliby dużo kłopotów.

Ostatecznie bezwzględnym wymogiem Twojej pracy jest przestrzeganie instrukcji od zaufanych firm do prowadzenia ich projektów. Czy generalnie nie jesteś w stanie odłożyć się na emerytów, jeśli istnieje dobry powód, by cenić ich doświadczenie? Czy zamierzasz postępować zgodnie z instrukcjami, których nie rozumiesz? Czy uważasz, że świat powinien się zatrzymać, dopóki nie będziesz przekonany? Wartości te są niezgodne z pracą w zespole.

Ostatni punkt. Wróć do projektów napisanych sześć miesięcy temu. Pomyśl o projektach, które napisałeś na uniwersytecie. Zobacz, jak źle się teraz wydają - wszystkie błędy i odwrócone wzornictwo, martwe punkty i błędne abstrakcje? Co jeśli powiem ci, że za sześć miesięcy policzysz te same wady w pracy, którą dzisiaj wykonujesz? Czy to pomaga pokazać, ile martwych punktów jest w twoim obecnym podejściu? Ponieważ taka jest różnica w doświadczeniu.

stale dodaje komentarze do mojego kodu, które sugerują, żebym wykonał o wiele więcej pracy niż jest to konieczne.

Możesz dawać rekomendacje, ale ostatecznie to nie twoja rola decyduje, co jest konieczne. Twoim zadaniem jest wdrożyć to, co zarząd lub (w tym przypadku twój recenzent) uzna za konieczne. Jeśli nie zgadzasz się z tym, co jest konieczne zbyt mocno lub zbyt mocno, prawdopodobnie nie znajdziesz pracy. W związku z tym częścią twojego profesjonalizmu jest pogodzenie się z tym i zachowanie pokoju.

Zasugerował, że zajmę się niejasną sprawą, która jest bardzo mało prawdopodobna

Są tutaj inne świetne odpowiedzi, które pokazują, że nawet nie-błędy (tj. Coś, co może nigdy nie zawieść ) nadal powinny być czasem przerabiane. (np. w przypadku budowania w przyszłości bezpieczeństwa produktu, przestrzegania standardów itp.) Wielką rolę programisty polega na tym, aby mieć jak największą pewność, że Twój kod będzie niezawodny w każdej możliwej sytuacji za każdym razem i w przyszłości - również zabezpieczony, nie tylko pracujący w testowanych sytuacjach w obecnych warunkach przez większość czasu

Sugestie dla recenzentów kodu, aby zwiększyć użyteczność biznesową recenzji kodu (Ty jako OP powinieneś zaproponować taką zmianę):

• Oznacz swoje komentarze według rodzaju. „Krytyczny” / „Obowiązkowy” / „Opcjonalny” / „Sugerowane ulepszenia” / „miło mieć” / „Rozmyślam”.

  Jeśli wydaje się to zbyt CDO / analne / skomplikowane, użyj co najmniej 2 poziomów: „Musisz naprawić, aby przejść przegląd i mieć możliwość scalenia zmiany” / „Wszystkie inne”.

Sugestie dotyczące obsługi sugestii przeglądu kodu, które wydają się mniej istotne, aby:

• Stwórz otwarty bilet w wybranym przez siebie systemie biletowym (twój zespół ma go, mam nadzieję?), Śledząc sugestię

• Umieść bilet # jako komentarz odpowiedzi do pozycji przeglądu kodu, jeśli twój proces pozwala na odpowiedzi na komentarze takie jak Rybie oko lub recenzje e-mailem.

• Skontaktuj się z recenzentem i wyraźnie zapytaj, czy ten element jest typu „należy naprawić, czy nie zostanie scalony / wydany”.

  • Jeśli odpowiedź brzmi „Tak”, ale nie zgadzasz się, pozwól osobie odpowiedzialnej za zarządzanie projektem (PM, kierownik, itp.) Podjąć decyzję - przedstaw w pełni i uczciwie nieporozumienie. Nie chodzi o to, które z was jest „właściwe”, ale o to, co jest lepsze dla projektu, więc praca PM / kierownika.

Teraz traktuj ten bilet jak każde inne żądanie programowania.

1. Jeśli po eskalacji postanowiono, że będzie to pilne, potraktuj to jak każdą pilną prośbę dewelopera. Zdepreoretuj inne prace i pracuj nad tym.

2. W przeciwnym razie pracuj nad nim zgodnie z priorytetem, który został przypisany i jego ROI (które mogą się różnić w zależności od linii biznesowej, jak wyjaśniono w innych odpowiedziach).

Należy nie eskalować to do zarządzania.

W większości firm menedżerowie zawsze decydują się nie pisać tego dodatkowego testu, nie tracić czasu na nieznaczną poprawę jakości kodu, aby nie tracić czasu na refaktoryzację.

W wielu przypadkach jakość kodu zależy od niepisanych reguł w zespole programistów i dodatkowego wysiłku włożonego przez programistów.

Jesteś młodszym programistą i jest to Twoja pierwsza recenzja kodu . Musisz zaakceptować radę i wykonać pracę. Możesz usprawnić przepływ pracy i zasady w zespole tylko wtedy, gdy najpierw je znasz i szanujesz, aby zrozumieć, dlaczego one istnieją. W przeciwnym razie będziesz nowym facetem, który nie przestrzega zasad i staje się samotnym wilkiem w drużynie.

Jesteś nowy w zespole, postępuj zgodnie ze wskazówkami, które otrzymujesz, dowiedz się, dlaczego one tam są, nie podawaj pierwszych wskazówek, które można zakwestionować na spotkaniu scrumowym. Rzeczywiste priorytety biznesowe staną się dla ciebie oczywiste po pewnym czasie bez pytania (i może nie być tym, co facet od zarządzania powie ci osobiście).

Bardzo ważne jest, abyś tworzył kod, który spełnia wymagania twojego żądania wiodącego / zarządzania. Każdy inny szczegół byłby po prostu „miło mieć”. Jeśli jesteś ekspertem (czytaj: „jeśli nie jesteś młodszym programistą”) w swojej dziedzinie, to jesteś „uprawniony” do rozwiązywania drobnych problemów, które napotkasz po drodze, bez konsultacji z liderami za każdym razem.

Jeśli uważasz, że coś jest nie tak i jesteś względnie ekspertem w swojej dziedzinie, masz duże szanse, że masz rację .

Oto kilka stwierdzeń, które mogą Ci się przydać:

• Poproszono mnie o zrobienie X, recenzent kodu sugeruje również wykonanie Y, czy powinienem to zrobić, czy powinienem przejść do ważniejszych rzeczy?
• Sugerujesz mi Y, więc czy możesz wymyślić co najmniej jeden przypadek testowy, który wychwyciłby to zachowanie, abym mógł je przetestować? Wierzę, że ten kod nie zostanie wywołany.
• Może nie powinniśmy najpierw opracować bezpiecznego rozwiązania awaryjnego dla odkrytych przypadków? Więc łapiemy je wcześnie i naprawiamy w ruchu, abyśmy mogli skupić się na ważniejszych rzeczach.
• OK, kiedy wdrażam Y, czy możesz być tak uprzejmy, aby napisać dla niego kilka przypadków testowych, abyśmy załatwili sprawę raz na zawsze ?
• Poproszono mnie o wykonanie X i myślę, że mógłbym wykonać Y, chyba że istnieją inne priorytety. Następnym razem, dlaczego nie zgłosisz żądania funkcji zamiast wstawić go jako komentarz do recenzji mojego kodu ? Przynajmniej możemy usłyszeć dalsze opinie innych członków zespołu na temat tej funkcji przed jej wdrożeniem (generalnie wszelkie ważne rzeczy powinny być cechą i powinny być obsługiwane przez więcej niż jedną osobę; zwykle przegląd kodu powinien dotyczyć przeglądu kodu i rozwiązań rozwiązań; reszta naprawia błędy i funkcje).

Czy poważnie myślisz, że postawa recenzenta szkodzi projektowi, czy też uważasz, że ma on większość czasu (chyba że czasami popełnił drobne błędy w ocenie i przesadził)?

Dla mnie brzmi to bardziej, jak pisze rzeczy, które nie należą do recenzji kodu, co jest złą praktyką, ponieważ utrudnia wszystkim śledzenie rzeczy. Nie wiem jednak, jakie inne komentarze do recenzji napisał, więc nie mogę nic powiedzieć o innych sprawach.

Ogólnie staraj się unikać obu następujących elementów:

• Nie robisz tego, o co prosiłeś
• Sprawiasz, że twój recenzent jest głupi

Jeśli rzeczywiście sprawdza Twój kod, to dlatego, że kierownictwo ufa mu bardziej niż tobie.

Jeśli podczas przeglądu kodu nie ma zgody co do zakresu:

1. Dokumentuj zakres, który jest faktycznie objęty kodem. Nikt nie lubi przykrych niespodzianek.
2. Zrozum, że zakres jest decyzją biznesową. Zakres powinien być już znany przed rozpoczęciem pracy nad funkcją, ale jeśli nie, zawsze możesz poprosić o wyjaśnienie później.

Jeśli recenzent kodu podejmuje decyzje biznesowe, może zmienić zakres w dowolnym momencie, nawet podczas przeglądania kodu, ale nie robi tego w roli recenzenta kodu.

Jeśli nie możesz udowodnić, że przypadek na krawędzi jest niemożliwy, musisz założyć, że jest to możliwe. Jeśli jest to możliwe, nieuniknione jest, że w końcu to nastąpi, a raczej wcześniej niż później. Jeśli przypadek krawędzi nie wystąpił podczas testowania, może to wskazywać, że zasięg testu jest niepełny.

1. Zaakceptuj informacje zwrotne.
2. Przed wprowadzeniem jakichkolwiek zmian w kodzie postaraj się zbudować test dla przypadku krawędzi i sprawdź, czy możesz uzyskać błąd testu (dowód na istnienie problemu). Jeśli niemożliwe jest utworzenie takiego przypadku testowego i uzyskanie błędu testowego, możesz dojść do wniosku, że przypadek skrajny jest w rzeczywistości niemożliwy (chociaż wahałbym się wyciągnąć taki wniosek).
3. Jeśli możesz uzyskać błąd testu, zastosuj odpowiednią zmianę kodu, aby przejść test.

Dla dobra produktu prawdopodobnie prawdopodobnie będziesz w stanie wyprodukować obudowę krawędzi i wywołać awarię, abyś mógł zastosować poprawkę i mieć pewność, że potencjalny problem został rozwiązany.

Głównym celem recenzji kodu jest dodatkowe spojrzenie na kod. Nikt z nas nie jest odporny na błędy i niedopatrzenia. To zbyt często spoglądać na fragment kodu wiele razy i nie zauważać oczywistego błędu, w którym świeża para oczu może go natychmiast odczytać.

Jak powiedziałeś, zaimplementowałeś nowy algorytm. Rozsądne byłoby wyciąganie wniosków na temat zachowania nowego algorytmu na podstawie zachowania lub obserwacji jego poprzednika.

Są recenzenci kodu, którzy wiedzą, co robią, a jeśli twierdzą, że coś musi zostać zmienione, to trzeba to zmienić, a kiedy mówią, że coś musi zostać przetestowane, to musi zostać przetestowane.

Są recenzenci kodu, którzy muszą uzasadnić swoje istnienie, tworząc niepotrzebną pracę dla innych.

Który z nich należy do ciebie, i jak poradzić sobie z drugim rodzajem, jest bardziej kwestią dla miejsca pracy. Wymiana stosów.

Jeśli używasz scrum, to pytanie brzmi, czy twoja praca wykonuje to, co powinna (najwyraźniej tak), i możesz umieścić obsługę bardzo rzadkiego i być może niemożliwego przypadku na zaległości, gdzie będzie on traktowany priorytetowo, i czy przechodzi do sprintu, a następnie recenzent może go odebrać i wykonać 13 godzin pracy. Jeśli wykonujesz zadanie X, a ponieważ wykonujesz zadanie X, zdajesz sobie sprawę, że zadanie Y również musi zostać wykonane, to zadanie Y nie staje się częścią zadania X, jest to jego samodzielna praca.