Jestem wielkim zwolennikiem czystego kodu i kunsztu, ale obecnie jestem na stanowisku, w którym nie jest to uważane za najwyższy priorytet. Czasami znajduję się w sytuacji, gdy kod peera jest pełen bałaganu i bardzo mało uwagi na temat przyszłej konserwacji, chociaż jest funkcjonalny i nie zawiera żadnych błędów.
Jak radzisz sobie z sugerowaniem ulepszeń w przeglądzie kodu, jeśli uważasz, że jest tak wiele rzeczy, które wymagają zmiany, a nadchodzi termin? Należy pamiętać, że zasugerowanie ulepszeń po upływie terminu może oznaczać, że zostaną one całkowicie pozbawione priorytetów, gdy pojawią się nowe funkcje i poprawki błędów.
Odpowiedzi:
Sprawdź dokładnie swoją motywację. Jeśli uważasz, że kod powinien zostać zmieniony, powinieneś być w stanie podać jakiś powód, dla którego uważasz, że należy go zmienić. Powód ten powinien być bardziej konkretny niż „zrobiłbym to inaczej” lub „jest brzydki”. Jeśli nie możesz wskazać na jakąś korzyść wynikającą z proponowanej zmiany, to nie ma sensu poświęcać czasu (inaczej pieniędzy) na jej zmianę.
Każda linia kodu w projekcie jest linią, którą należy zachować. Kod powinien być tak długi, jak to konieczne, aby wykonać zadanie i być łatwy do zrozumienia, i już nie. Jeśli możesz skrócić kod bez utraty przejrzystości, to dobrze. Jeśli możesz to zrobić, zwiększając przejrzystość, to znacznie lepiej.
Kod jest jak konkret: trudniej go zmienić po pewnym czasie oczekiwania. Jeśli to możliwe, zasugeruj swoje zmiany wcześniej, aby zminimalizować koszty i ryzyko zmian.
Każda zmiana kosztuje. Przepisanie kodu, który działa i prawdopodobnie nie wymaga zmiany, może być zmarnowanym wysiłkiem. Skoncentruj swoją uwagę na sekcjach, które bardziej mogą ulec zmianie lub które są najważniejsze dla projektu.
Forma podąża za funkcją, a czasem odwrotnie. Jeśli kod jest niechlujny, istnieje większe prawdopodobieństwo, że zawiera również błędy. Poszukaj tych błędów i krytykuj wadliwą funkcjonalność, a nie estetyczny wygląd kodu. Zaproponuj ulepszenia, które poprawią działanie kodu i ułatwią jego weryfikację.
Rozróżnij projekt i wdrożenie. Ważna klasa z kiepskim interfejsem może rozprzestrzeniać się w ramach projektu takiego jak rak. Nie tylko obniży to jakość pozostałej części projektu, ale także zwiększy trudność naprawy szkód. Z drugiej strony klasa z dobrze zaprojektowanym interfejsem, ale kiepską implementacją, nie powinna być wielkim problemem. Zawsze możesz ponownie wdrożyć klasę, aby uzyskać lepszą wydajność lub niezawodność. Lub, jeśli działa poprawnie i jest wystarczająco szybki, możesz zostawić go w spokoju i mieć pewność, że jego cruft jest dobrze zamknięty.
Podsumowując wszystkie powyższe punkty: Upewnij się, że proponowane zmiany stanowią wartość dodaną.
źródło
Jest dobry punkt do dodawania wartości poprzez refaktoryzację. Zmiany muszą osiągnąć trzy rzeczy:
Uwagi:
źródło
Jeśli kod działa bez poważnych błędów i zbliża się główny termin (jak w przypadku zysków i strat lub PR korporacyjnego), jest już za późno, aby zaproponować ulepszenia wymagające poważnych zmian. Nawet ulepszenia kodu mogą stwarzać ryzyko dla wdrożenia projektu. Czas na ulepszenia był wcześniejszy w projekcie, kiedy było więcej czasu na zainwestowanie w przyszłą niezawodność bazy kodu.
źródło
Przegląd kodu służy 3 celom:
Sprawdzanie błędów
Sprawdzanie, gdzie można poprawić kod
Narzędzie do nauczania dla każdego, kto napisał kod.
Ocena jakości projektu / kodu dotyczy oczywiście 2 i 3.
Aż do # 2:
Wyjaśnij BARDZO jasno, jakie są korzyści z proponowanych zmian a koszty do naprawienia. Jak każda decyzja biznesowa, powinna dotyczyć analizy kosztów i korzyści.
Np. „Podejście do projektowania X znacznie zmniejszyłoby prawdopodobieństwo wystąpienia błędu Y podczas wykonywania zmiany Z, i wiemy, że ten fragment kodu podlega zmianom typu Z co 2 tygodnie. Koszt obsługi przerwy w produkcji z powodu błędu Y + znalezienie błędu + naprawienie i zwolnienie poprawki + koszt alternatywny za niedostarczenie następnego zestawu rzeczy jest
$A
; podczas gdy koszt czyszczenia kodu teraz i koszt alternatywny (np. cena spóźnionej wysyłki lub z mniejszą liczbą funkcji) jest$B
. Teraz oceń - a raczej poproś swojego lidera / kierownika zespołu - oceń$A
vs$B
i zdecyduj.Pomoże to inteligentnemu zespołowi w skutecznym zarządzaniu tym. Np. Podejmą racjonalną decyzję na podstawie PEŁNYCH informacji
To (szczególnie jeśli dobrze to powiesz) podniesie TWÓJ status - np. Jesteś kimś na tyle inteligentnym, aby dostrzec korzyści płynące z lepszego projektowania, I na tyle inteligentnym, aby nie wymagać tego z religijnego punktu widzenia bez rozważania kwestii biznesowych.
ORAZ, w prawdopodobnym przypadku wystąpienia błędu Z, zyskujesz o wiele większą dźwignię na następnym zestawie sugestii.
Aż do # 3:
źródło
Wybierz swoje bitwy, jeśli zbliża się termin, nie rób nic. Następnym razem, gdy ktoś inny będzie przeglądał lub utrzymywał kod, a on nadal ma z tym problem, podejdź do niego z pomysłem, że jako zespół powinieneś bardziej skupić się na jakości kodu podczas przeglądania kodu, abyś nie miał więcej problemów później.
Powinny zobaczyć wartość przed wykonaniem dodatkowej pracy.
źródło
Mój komentarz zawsze zaczynam od „chciałbym”, co sygnalizuje, że mój jest po prostu jednym z wielu poglądów.
Zawsze też podam powód.
„ Chciałbym wyodrębnić ten blok do metody ponieważ czytelności.”
Komentuję wszystko; duży i mały. Czasami robię ponad sto komentarzy na temat zmiany, w takim przypadku polecam również programowanie w parach i oferuję się jako skrzydłowy.
Z różnych powodów próbuję ustanowić wspólny język; czytelność, DRY, SRP itp.
Stworzyłem również prezentację na temat czyszczenia kodu i refaktoryzacji, wyjaśniającą dlaczego i pokazującą, jak to zrobiłem moim kolegom. Do tej pory trzymałem go trzy razy, a firma konsultingowa, z której korzystamy, poprosiła mnie o ponowne zatrzymanie go dla nich.
Ale niektórzy i tak nie będą słuchać. Potem pozostaje mi ciągnięcie rangi. Jestem szefem projektu. Jakość kodu jest moją odpowiedzialnością. Ta zmiana nie przejdzie w moim zegarku w jego obecnym stanie.
Pamiętaj, że jestem więcej niż chętny do wycofania się z każdego komentarza, który robię; z przyczyn technicznych, terminów, prototypów itp. Wciąż mam wiele do nauczenia się na temat kodowania i zawsze będę słuchać rozsądku.
Och, a ja niedawno zaproponował, aby kupić obiad w pierwszym dniu mojego zespołu, który złożył nietrywialne zmianę na którą nie miał żadnego komentarza. (Hej, ty też musisz się dobrze bawić. :-)
źródło
Ten kod jest gotowy. W pewnym momencie przeprojektowanie staje się zbyt kosztowne, aby uzasadnić. Jeśli kod działa już bez żadnych błędów lub nie ma żadnych błędów, będzie to niemożliwa sprzedaż. Zaproponuj kilka sposobów rozwiązania tego problemu w przyszłości i przejdź dalej. Jeśli / kiedy kod zepsuje się w przyszłości, wówczas ponownie ocenia wartość przeprojektowania. To może nigdy nie pęknąć, co byłoby świetne. Tak czy inaczej, jesteś w punkcie, w którym sensowne jest uprawianie hazardu, aby się nie złamał, ponieważ koszt będzie taki sam teraz lub później: wyciągnięty, straszny przeprojektowanie.
To, co musisz zrobić w przyszłości, to ściślejsze iteracje programistyczne. Jeśli byłeś w stanie przejrzeć ten kod, zanim zainwestowano całą pracę nad usunięciem błędów, warto zaproponować przeprojektowanie. Pod koniec nigdy nie ma sensu przeprowadzać poważnego refaktoryzacji, chyba że kod zostanie napisany w zasadniczo niemożliwy do utrzymania sposób i wiadomo na pewno, że kod będzie musiał zostać zmieniony wkrótce po wydaniu.
Biorąc pod uwagę wybór między dwiema opcjami (refaktoryzator lub brak refaktora), zastanów się, która brzmi jak mądrzejsza sprzedaż:
lub
Gdybyś powiedział jedno z nich, twój szef prawdopodobnie powiedziałby:
Najważniejsze jest to, że czasami trochę długu technicznego ma sens, jeśli nie byłeś w stanie naprawić pewnych wad, kiedy było tanie (wczesne iteracje). Projektowanie kodu jakości ma coraz mniejsze zyski, gdy zbliżasz się do ukończonej funkcji i terminu.
źródło
[Ta odpowiedź jest nieco szersza niż pierwotnie postawione pytanie, ponieważ jest to przekierowanie dla wielu innych pytań dotyczących recenzji kodu.]
Oto kilka zasad, które uważam za przydatne:
Krytykuj prywatnie, chwal publicznie. Poinformuj kogoś o błędzie w kodzie jeden na jednego. Jeśli zrobili coś genialnego lub podjęli się zadania, którego nikt nie chciał, pochwal je na spotkaniu grupowym lub w e-mailu wysłanym do zespołu.
Podziel się własnymi błędami. Opowiadam historię mojego fatalnego pierwszego przeglądu kodu (otrzymanego) studentom i młodszym kolegom. Dałem też uczniom do zrozumienia, że tak szybko złapałem ich robaka, ponieważ zrobiłem to sam. W przeglądzie kodu może to wyglądać następująco: „Myślę, że tutaj źle pomyliłeś się zmiennych indeksu. Zawsze to sprawdzam, ponieważ z powodu moich błędów indeksów i awarii centrum danych”. [Tak, to prawdziwa historia.]
Pamiętaj, aby robić pozytywne komentarze. Krótkie „miłe!” lub „zgrabna sztuczka!” może sprawić, że dzień młodszego lub niepewnego programisty.
Załóżmy, że druga osoba jest inteligentna, ale czasami nieostrożna. Nie mów: „W jaki sposób oczekujesz, że dzwoniący otrzyma wartość zwracaną, jeśli w rzeczywistości jej nie zwrócisz ?!” Powiedz: „Wygląda na to, że zapomniałeś instrukcji return”. Pamiętaj, że na początku pisałeś okropny kod. Jak ktoś kiedyś powiedział: „Jeśli nie wstydzisz się swojego kodu sprzed roku, nie uczysz się”.
Zachowaj sarkazm / wyśmiewanie dla przyjaciół spoza twojego miejsca pracy. Jeśli kod jest niesamowicie okropny, żartuj sobie z niego gdzie indziej. (Uważam, że wygodnie jest ożenić się z innym programistą.) Na przykład nie podzieliłbym się poniższymi kreskówkami (lub tym ) z moimi kolegami.
źródło
Kiedy łyżka cukru pomaga zejść lekarstwom, a to, co jest złe, można zwięźle wyrazić - nie ma 20 rzeczy źle - przedstawię formularz, który sugeruje, że nie mam udziałów, nie zainwestowałem ego w to, czego chcę być usłyszanym. Zwykle jest to coś takiego:
lub
Jeśli przyczyny są dość oczywiste, nie podam ich. Daje to innym ludziom szansę przejęcia intelektualnej własności sugestii, jak w:
„Tak, to dobry pomysł, ponieważ < Twój oczywisty powód tutaj >.”
Jeśli poprawa jest dość oczywista, ale nie na tyle, aby sprawić, że wyglądam na idiotkę, że o niej nie myślę, a powód, dla którego to zrobiłem, odzwierciedla wartość dzieloną ze słuchaczem, to czasem nawet tego nie sugeruję:
Zastanawiam się, czy istnieje sposób na ... <tutaj oświadczenie o wspólnej wartości>
To jest tylko do czynienia z naprawdę drażliwymi ludźmi - z większością moich rówieśników, po prostu pozwalam im to mieć!
źródło
Przeglądy kodu nie zawsze mają na celu wprowadzanie ulepszeń.
Recenzja pod koniec projektu, jak się wydaje, ma miejsce w tym przypadku, aby wszyscy wiedzieli, od czego zacząć szukać błędów (lub w przypadku lepiej zaprojektowanego projektu, który może być dostępny do ponownego użycia). Bez względu na wynik przeglądu po prostu nie ma czasu na nic.
Aby faktycznie wprowadzić zmiany, musisz omówić kod i projekt dużo wcześniej w projekcie - kod jest o wiele łatwiejszy do zmiany, jeśli istnieje tylko jako omówienie możliwych podejść.
źródło
Twoje pytanie brzmi: „Jak sprawdzić kod źle zaprojektowany kod?”:
Odpowiedź IMO jest prosta. Porozmawiaj o PROJEKCIE kodu i jak projekt jest wadliwy lub nie spełnia wymagań. Jeśli zauważysz wadliwy projekt lub „nie spełnia wymagań”, programista będzie zmuszony zmienić swój kod, ponieważ nie robi tego, co powinien.
Jeśli kod jest „funkcjonalnie wystarczający” i / lub „spełnia specyfikację” i / lub „spełnia wymagania”:
Jeśli jesteś rówieśnikiem tego dewelopera, nie masz bezpośredniej mocy, która pozwoliłaby mu „powiedzieć mu”, aby wprowadził zmiany.
Pozostało Ci kilka opcji:
Uważam, że nie ma srebrnej kuli. Musisz użyć wszystkich trzech i musisz być kreatywny w używaniu wszystkich trzech.
źródło
W przypadku okropnie złego projektu należy skupić się na maksymalizacji kapsułkowania . W ten sposób łatwiej jest zastąpić poszczególne klasy / pliki / podprogramy lepiej zaprojektowanymi klasami.
Skoncentruj się na zapewnieniu, że publiczne interfejsy komponentów są dobrze zaprojektowane, a wewnętrzne elementy robocze są starannie ukryte. Również opakowania do przechowywania danych są niezbędne. (Duże ilości przechowywanych danych mogą być bardzo trudne do zmiany, więc jeśli dostaniesz „upuszczanie implementacji” w inne obszary systemu, masz kłopoty).
Po uzyskaniu barier między komponentami skoncentruj się na komponentach, które mogą powodować poważne problemy.
Powtarzaj aż do ostatecznego terminu lub dopóki system nie będzie „idealny”.
źródło
Zamiast bezpośredniej krytyki czyjegoś kodu, zawsze lepiej jest być zachęcającym do komentowania podczas przeglądania kodu.
Jednym ze sposobów, który podążam jest
Takie uwagi będą traktowane poważnie, nawet jeśli zbliżają się terminy. I prawdopodobnie zostaną wdrożone w następnym cyklu rozwojowym.
źródło
Przegląd kodu musi być zintegrowany z kulturą i cyklem programowania, aby działał. Jest mało prawdopodobne, że zaplanowanie przeglądu dużego kodu pod koniec rozwoju funkcji X będzie działać. Przede wszystkim wprowadzanie zmian będzie trudniejsze, a ktoś prawdopodobnie poczuje się zawstydzony - tworząc opór wobec aktywności.
Powinieneś mieć wczesne i częste zatwierdzenia w połączeniu z recenzjami na poziomie zatwierdzenia. Po zainstalowaniu narzędzi do analizy kodu większość recenzji będzie szybka. Zautomatyzowane narzędzia do analizy / przeglądu kodu, takie jak FindBugs i PMD , pomogą Ci uzyskać dużą klasę błędów projektowych. Nie pomogą one jednak rozwiązać problemów na poziomie architektonicznym, więc musisz mieć solidny projekt i ocenić ogólny system na podstawie tego projektu.
źródło
Zwiększ jakość recenzji kodu.
Oprócz jakości sprawdzanego kodu istnieje coś takiego jak jakość samego przeglądu kodu:
Znacznie łatwiej jest zaakceptować dobrej jakości przegląd kodu niż niektóre głównie budzące wątpliwości ego grooming.
źródło
W pytaniu są dwie znaczące kwestie, część taktyczna i zbliżająca się data. Są to odrębne kwestie - pierwsza to kwestia komunikacji i dynamiki zespołu, druga to planowanie i ustalanie priorytetów.
Taktownie . Zakładam, że chcesz uniknąć szczotkowanych ego i negatywnych reakcji w stosunku do recenzji. Jakieś sugestie:
Druga część to ustalanie priorytetów . Masz wiele sugestii dotyczących ulepszeń, ale ponieważ zbliża się termin, jest tylko czas, aby zastosować kilka.
Po pierwsze, chcesz tego uniknąć, przede wszystkim! Robisz to, wykonując ciągłe, przyrostowe recenzje. Nie pozwól programistom pracować nad tygodniami nad funkcją, a następnie przejrzyj ją w ostatniej chwili. Po drugie, recenzje kodu i czas na wdrożenie sugestii przeglądu powinny być częścią regularnego planowania i szacunków dla każdego zadania. Jeśli nie ma wystarczająco dużo czasu na prawidłowe sprawdzenie, coś poszło nie tak w planowaniu.
Załóżmy jednak, że coś poszło nie tak, a teraz masz do czynienia z wieloma recenzjami i nie masz czasu na ich wdrożenie. Musisz ustalić priorytety. Następnie przejdź do zmian, które będą najtrudniejsze i najbardziej ryzykowne do zmiany później, jeśli je odłożysz.
Nazewnictwo identyfikatorów w kodzie źródłowym jest niezwykle ważne dla czytelności i łatwości konserwacji, ale jest również dość łatwe i wiąże się z niskim ryzykiem zmiany w przyszłości. To samo z formatowaniem kodu. Więc nie skupiaj się na tych rzeczach. Z drugiej strony, zdrowy rozsądek publicznie ujawnionych interfejsów powinien mieć najwyższy priorytet, ponieważ naprawdę trudno je zmienić w przyszłości. Trwałe dane trudno zmienić - jeśli najpierw zaczniesz przechowywać niespójne lub niekompletne dane w bazie danych, naprawdę trudno będzie je naprawić w przyszłości.
Obszary objęte testami jednostkowymi są obarczone niskim ryzykiem. Zawsze możesz to naprawić później. Obszary, które nie są, ale które można poddać testom jednostkowym, są mniej zagrożone niż obszary, których nie można poddać testom jednostkowym.
Załóżmy, że masz dużą część kodu bez testów jednostkowych i wszelkiego rodzaju problemów z jakością kodu, w tym na stałe zależną od usługi zewnętrznej. Zamiast tego wstrzykiwanie tej zależności powoduje, że fragment kodu jest testowalny. Oznacza to, że możesz w przyszłości dodawać testy, a następnie pracować nad naprawieniem pozostałych problemów. W przypadku stałej zależności nie można nawet dodawać testów. Najpierw skorzystaj z tej poprawki.
Ale staraj się przede wszystkim nie dopuścić do tego scenariusza!
źródło