Jaki jest najlepszy sposób dla programisty na sprawdzenie kodu?

Jestem dość nowy w recenzowaniu kodu, ale pisałem od lat podczas mojej doktoratu - co nie zawsze czyni cię dobrym programistą!

Jeśli recenzent zmieni kod i przejdzie z tobą wiersz po wierszu, co zrobisz, jeśli się nie zgodzisz? Czasami dokonałeś wyboru X, a recenzent zmienił go na Y i miałeś Y na myśli, ale zdecydowałeś się tego nie robić z powodu wad, ale recenzent twierdzi, że te wady nie są ważne. Czy powinieneś werbalizować swoje nieporozumienie, czy po prostu nie i słuchać ich?

Jest to trudne, jeśli nie jesteś dobry w przyjmowaniu krytyki i jeśli recenzent jest osobą starszą w organizacji. Nie byłoby dobrze uważać się za zbyt defensywnego.

To prawda, że ​​obrona jako defensywna nie jest pomocna; ale potem - żadna z nich nie jest krytykowana, więc jeśli czujesz, że tak się dzieje, naprawdę powinieneś wyrazić swoje obawy, że przegląd kodu nie pomaga w pisaniu lepszego kodu w organizacji.

Recenzent ma obowiązek sprawdzić kod pod kątem prawdziwej niezgodności i wad, nie używać go jako sposobu na napisanie kodu w sposób, w jaki zrobiłby to. Oznacza to, że recenzja ma na celu sprawdzenie, w jaki sposób coś zrobiłeś, i wskazanie obszarów, w których popełniłeś błąd (coś, co robimy najlepiej z nas) lub nie zrozumiałeś ram, standardów lub „historycznych powodów”, że jakiś kod jest napisany tak jak tam jesteś.

Więc jeśli istnieje wiele sposobów zrobienia czegoś, musi istnieć dobry powód, dla którego twój kod został zmieniony na alternatywny sposób, w przeciwnym razie jego po prostu niekonstruktywna rezygnacja, która ci nie pomoże.

Pamiętaj też, że recenzent też nie jest doskonały. Mogą mieć pomysł, że Y jest na to sposób i nie zdawali sobie sprawy, że X jest lepszy. Powinieneś wyjaśnić powody, dla których zrobiłeś to w sposób X. Recenzent może się z tobą zgodzić, lub może powiedzieć, dlaczego Y jest lepszym rozwiązaniem - mogą istnieć inne czynniki, o których nie wiesz, że tak robi.

Krótko mówiąc, recenzje są sposobem na zachęcenie członków zespołu do komunikowania się na temat zmian w kodzie. Porozmawiaj o tym z recenzentem.

JEŻELI to pomaga, mów bezstronnie, jakbyś nawet nie był autorem recenzowanego kodu. W końcu kod należy do firmy lub zespołu, a zespół będzie musiał go zachować. Właśnie byłeś facetem, który to napisał, to nie jest tak ważny czynnik, jak wielu uważa.

Pisanie kodu komputerowego jest doskonałym przykładem podejmowania decyzji w warunkach niepewności . Zawsze występują sprzeczne naciski, a sposób, w jaki decydujesz w danym pytaniu, zależy od tego, jakie postrzegasz presje i jak duże je postrzegasz.

Dlatego, gdy recenzent nie zgadza się z twoją decyzją, oznacza to, że widzi inne presje / ryzyko niż ty, lub uważa niektóre z nich za większe / mniejsze niż ty. Musisz bezwzględnie mówić o tych różnicach, ponieważ ich brak utrwala problemy, które doprowadziły przede wszystkim do nieporozumień.

Jeśli Twój recenzent jest starszy, jego doświadczenie może poprawnie powiedzieć mu, że to lub inne ryzyko nie jest zbyt istotne w praktyce, lub może wiedzieć, że jakiś błąd ma długą historię w Twojej organizacji i warto zachować szczególną ostrożność aby tego uniknąć. I odwrotnie, jeśli czujesz, że wiesz coś, czego prawdopodobnie twój recenzent nie wie, musisz to absolutnie wyrazić; milczenie oznacza z twojej strony zwolnienie z obowiązku.

Najważniejszą częścią radzenia sobie z sytuacją jest zrozumienie, że krytyka decyzji projektowych praktycznie zawsze nie jest krytyką czyjejś osobowości. (W rzadkich przypadkach, gdy tak naprawdę jest, zauważysz, że wkrótce, a jeśli naprawdę nie możesz kogoś zadowolić, nic, co robisz, nie robi żadnej różnicy, więc gdzie jest szkoda w stosowaniu najlepszych praktyk? Znacznie lepiej znaleźć lepszą pozycję jako tak szybko, jak to możliwe.) Jest to po prostu wynik różnych sytuacji, w których różne osoby postrzegają wiele rodzajów ryzyka i korzyści związanych z kodem komputerowym, a biorąc pod uwagę złożoność nowoczesnego kodu komputerowego, należy się tego spodziewać. Mówienie o różnicach pomaga ulepszyć kod i poprawić współpracę w tym przypadku i w przyszłych przypadkach.

Pozostałe odpowiedzi zawierają już bardzo dobre informacje. Chciałbym trochę rozwinąć niektóre aspekty, o których wspomniał gbjbaanb (zobacz mój komentarz do jego odpowiedzi).

Z mojego doświadczenia wynika, że ​​podczas recenzji kodu obserwowałem różne informacje zwrotne:

1. „Szczerze wierzę, że jest to złe / nieoptymalne i że należy to zmienić w ten sposób”. Zwykle poważnie traktuję tego rodzaju informacje zwrotne i sprzeciwiam się sugerowanej zmianie tylko wtedy, gdy uważam, że mam na to mocną stronę.
2. „Uważam, że twoje rozwiązanie jest OK, rozważ tę alternatywę, ale to od Ciebie zależy, czy zaakceptujesz zmianę, czy nie.” Poważnie podchodzę do tego rodzaju opinii: recenzent sugeruje alternatywę, nawet jeśli nie ma silnej opinii, że ich rozwiązanie jest lepsze. Mam okazję się czegoś nauczyć i zaakceptuję zmianę, jeśli bardziej mi się spodoba. W przeciwnym razie recenzent uzna to za właściwe, jeśli zachowam kod zgodnie z moim gustem.
3. „Zrobiłbym to inaczej, więc trzeba to zmienić, kropka.”, A nawet „Och, zmieniłem kod, ponieważ ...”, zmiana nie została zasugerowana, została już zatwierdzona! Podane jest krótkie wyjaśnienie dotyczące zmiany, z sugestią, że nie ma zbyt wiele czasu na omówienie szczegółów, ponieważ musimy przejść do następnego zadania.
4. Recenzent sugeruje małe zmiany o trywialnym charakterze, które nie poprawiają kodu, ale po prostu go zmieniają. Poproszony o omówienie sugerowanych zmian, recenzent wdaje się w długie dyskusje na temat trywialnych szczegółów, dopóki nie zrezygnujesz z wyczerpania.

Opcje 3, 4 mogą być zamaskowane jako 1 i 2: „Naprawdę ważne było, aby zrobić to tak, jak zasugerowałem”. lub „Możesz odmówić zmiany, jeśli naprawdę tego chcesz”, powiedział tonem, który w rzeczywistości oznacza przeciwieństwo tego, co się mówi. W przypadku sprzeciwu wobec zmian, które uważasz za niepotrzebne, własność współdzielonego kodu można wykorzystać jako uzasadnienie takiego podejścia: „Kod nie należy do ciebie, należy do zespołu!” Możesz powiedzieć, kiedy intencja recenzenta nie jest uczciwa, jeśli recenzent nie jest bardzo otwarty na dyskusję, denerwuje się i próbuje za wszelką cenę forsować swoje rozwiązanie. Zasadniczo już zdecydowali, a dalsza dyskusja to tylko strata czasu.

Opcje 1 i 2 IMO są oznaką uczciwego recenzenta, który stara się pomóc, próbuje cię czegoś nauczyć, szanując twoją pracę, a także jest otwarty na naukę czegoś sam. W tym scenariuszu staram się nie być zbyt dumny, gdy otrzymuję konstruktywną opinię od recenzenta.

Opcje 3, 4 sugerują, że trwa pewna gra o władzę: ważne jest to, czy robimy to po swojemu, czy po swojemu, a nie, że staramy się znaleźć dobre rozwiązanie, które spełnia oba. Może to być związane z ego recenzenta, ale także z niemożnością zrozumienia kodu, który nie jest zgodny z ich sposobem myślenia. Zazwyczaj są zakłócane przez kod, który nie wygląda im znajomo i chcieliby narzucić swoją drogę na całą bazę kodu.

Jeśli sytuacje 3 i 4 zdarzają się zbyt często, praca zespołowa może stać się dość nieprzyjemna. W takiej sytuacji rozważałbym odejście z zespołu.

Aby rozwiązać problem dotyczący postępowania w przypadku braku zgody ...

Mówiąc o tym, jest to najlepsza droga, jak większość ludzi już zauważyła.

Jeśli już to zrobiłeś, może więcej niż jeden raz, jedną z przydatnych technik, których używamy, jest to, że nadal istnieje spór w niektórych obszarach, to powiedzieć „tak, dobrze byłoby zrewidować, że -

Czy możemy wystawić na to osobny bilet?

Oddzielny bilet umożliwia wprowadzenie kodu zamiast przerażającego trybu „rezygnacji i nigdy nie ruszaj się”, który dobrze znałem w niektórych miejscach. To dobrze pasuje do zwinności, wykonując najmniejszą możliwą ilość i rozkładając ładunek. Czasami yagni kończy się aplikowaniem. Czasami menedżer produktu zdecyduje, że istnieje pilniejsza potrzeba niż ten refaktor pod względem wartości dla klienta, terminów i zasobów.

Przegląd kodu jest prawdopodobnie ogólnie dobrą rzeczą, ale z mojego doświadczenia najlepiej jest służyć jako narzędzie dla programistów w nowych zespołach korzystających z nowych wytycznych kodowania lub do naprawy ważnych błędów, aby zmniejszyć ryzyko popełnienia błędów. Jeśli uważasz, że wiesz lepiej niż recenzent, powinieneś zapytać, dlaczego proponowane przez niego rozwiązanie jest lepsze, i argumentować swoje spostrzeżenia na ten temat. Nikt nie wie wszystkiego najlepiej, więc przegląd kodu powinien lub może być cennym doświadczeniem w nauce dla obu.

Przegląd kodu jest zarówno okazją do wychwycenia potencjalnych problemów, jak i do przekazania wiedzy zarówno recenzentowi, jak i programistowi.

Jako recenzent kodu odpowiedzialny jest za podkreślenie możliwych obszarów ryzyka, niezgodności ze standardową praktyką, ulepszeń i ogólnie po prostu innego punktu widzenia na ten sam obszar kodu.

Nie powinno to skutkować zmianą kodu bez omawiania / rozumienia decyzji programistów w momencie opracowywania.

Jeśli recenzent wprowadza zmiany, może mieć trudności z przekazaniem pracy innym, co jest trudne dla wielu inteligentnych ludzi.

Jako programista otrzymujący recenzję, jesteś chroniony przed możliwym problemem po wdrożeniu, masz szansę dowiedzieć się czegoś nowego oraz możliwość podzielenia się swoją wiedzą z recenzentem.

Niezależnie od stażu pracy, twój sposób myślenia może wymyślić rozwiązanie, które po prostu nie przydarzy się niektórym, więc recenzja może być okazją, by zabłysnąć, robiąc to, co uważasz za słuszne.

Dopóki zarówno programista, jak i recenzent zaakceptują, że mogą się mylić i że jest w porządku, że się myli, recenzja staje się czymś, co wzmacnia zespół, ponieważ pracujesz razem nad rozwiązaniem.

Wygląda na to, że Twój kod nie został jeszcze sprawdzony :-)

Celem przeglądu kodu jest uzyskanie kodu o przyzwoitej jakości i wiedza, że ​​masz kod o przyzwoitej jakości. Po przejrzeniu kodu niedoświadczonego programisty można go użyć do nauczenia pisania lepszego kodu, unikając jednocześnie frustracji tego programisty.

Recenzent nigdy nie powinien zmieniać kodu. Mogą przedstawiać mniej lub bardziej wyraźne sugestie, w jaki sposób chcieliby zmienić kod, i mogą zdecydować, czy przyjąć kod, czy nie.

Jeśli opinia idzie w prawo / gdybym przeglądu kodu, co będzie prawdopodobnie uzyskać pewne komentarze jak ja byłoby napisać kod, który można dowiedzieć się z, lub zignorować - to są rzeczy, gdzie mam swoje zdanie i jesteś wolny, aby mieć inne zdanie. W moim obszarze dobre nazewnictwo funkcji, zmiennych i tak dalej jest uważane za ważne, więc możesz otrzymać sugestie dotyczące ulepszenia nazewnictwa. Zwykle powinieneś wprowadzić zmiany w tym przypadku (czasem przez znalezienie jeszcze lepszej nazwy czegoś). Czasami znajdę błędy. Naprawiasz je. Czasami znajdę rzeczy, które uważam za błędy, i się mylę. Jeśli trudno jest zobaczyć, czy kod jest poprawny, uczynisz go bardziej oczywistym, poprawnym. Jeśli popełniłem błąd, powiedz mi.

Jeśli uważam, że projekt ogólnie nie jest poprawny, to powinno to zostać omówione wcześniej. Powinniśmy wtedy pomyśleć o tym, czy twój projekt jest wystarczająco dobry, biorąc pod uwagę, ile pracy wiąże się ze zmianą, a może po prostu się myliłem i twój projekt jest lepszy. Powinniśmy skończyć na porozumieniu.

Jeśli recenzent i recenzent nie mogą się zgodzić, mamy problem. Ponieważ oznacza to, że jedno z nas nie jest zdolne do pracy zespołowej, lub jedno z nas nie jest w stanie odróżnić dobrego lub złego projektu, lub obu. To niekoniecznie twoja wina. Niestety są programiści, którzy są starsi i nieświadomi, a to będzie problem dla firmy i dla ciebie.

Jeśli tak się stanie, pomyśl bardzo, bardzo mocno: Czy masz problem z zaakceptowaniem uzasadnionej krytyki? W takim przypadku musisz zmienić swoje nastawienie. Czy jesteś zbyt niedoświadczony, aby zrozumieć, dlaczego recenzent ma rację? Jeśli tak jest, to nie ma problemu. Zaufaj recenzentowi i dowiedz się. Czy na pewno znasz się lepiej niż recenzent? Zaakceptuj recenzję, ale zapytaj trzeciego zaufanego programistę o jego opinię. Pamiętaj, że możesz być naprawdę pewny siebie i mieć rację, ale możesz także być naprawdę pewien siebie i się mylić.