Jak mogę taktownie zasugerować ulepszenia źle zaprojektowanego kodu innych podczas przeglądu?

130

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.

Yony
źródło
25
Po pierwsze, upewnij się, że twój pogląd nie jest subiektywny. Dość często programiści odpisują kod innych ludzi, ponieważ po prostu lubią inny styl lub dialekt. Jeśli tak nie jest, staraj się oferować ulepszenia pojedynczo.
Koder
Ponieważ wiele prac związanych z tworzeniem oprogramowania wiąże się z kompromisami, a ponieważ mówię o kodzie, który opiera się głównie na projektowaniu, wiele komentarzy do recenzji kodu kończy się subiektywnie. Dlatego zapytałem „jak radzisz sobie z sugerowaniem ulepszeń”. Na pewno nie moim celem jest dyktowanie czegokolwiek.
Yony,
5
Pytanie brzmi, jakby przegląd kodu miał miejsce po zakończeniu testów. W takim przypadku tracisz czas na testowanie (wszelkie zmiany wymagają ponownego przetestowania) lub utrudniasz wprowadzanie zmian w wyniku przeglądu kodu (po co zmieniać kod, który już działa?).
vaughandroid
3
@Baqueta - Po co sprawdzać kod i marnować czas wielu ludzi, jeśli nie masz pojęcia, czy to jeszcze działa?
Dunk
4
@Baqueta Istnieją oczywiście różne rodzaje testów. Jeśli mają być przydatne, recenzje kodu powinny następować po początkowych testach, takich jak testy jednostkowe (abyś wiedział, że to działa), a przed testami końcowymi, takich jak testy akceptacyjne użytkownika (aby zmiany nie powodowały biurokracji) .
Caleb

Odpowiedzi:

160
  1. 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ę.

  2. 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.

  3. 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.

  4. 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.

  5. 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ę.

  6. 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ą.

Caleb
źródło
3
Rzeczywiście, sprowadza się to do „sprzedaży” twoich obaw. Jak wspomniano: zwróć uwagę na korzyści i wartość dodaną. To trudna praca, także z mojego doświadczenia.
Wivani,
4
Zrozumienie własnej motywacji to nie tylko sprzedaż. Musisz zrozumieć, dlaczego lubisz niektóre techniki, a inne nie, abyś wiedział, kiedy twoje podstawowe zasady są ważne, a kiedy nie. Wiele, wiele problemów wynika z tego, że doświadczeni programiści stosują prawidłowe techniki w niewłaściwych sytuacjach.
Jørgen Fogh
1
Twoje punkty sprawiają, że gra w golfa jest w porządku ... :-)
Florian Margaine
2
+1 za całą odpowiedź, ale szczególnie za „Jeśli kod jest nieuporządkowany, istnieje większe prawdopodobieństwo, że zawiera on także błędy”
Konamiman
2
Konsekwencją punktu (6), co ciekawe, wydaje się być fakt, że jakość interfejsu jest ważniejsza niż jakość implementacji
Brad Thomas
16

Jest dobry punkt do dodawania wartości poprzez refaktoryzację. Zmiany muszą osiągnąć trzy rzeczy:

  • popraw kod, który prawdopodobnie się zmieni
  • zwiększyć jasność
  • najmniejszy wysiłek

Uwagi:

  1. Wiemy, że czysty kod jest tańszy w pisaniu i utrzymaniu oraz przyjemniejszy w pracy. Twoim zadaniem jest sprzedać ten pomysł ludziom w Twojej firmie. Myśl jak sprzedawca, a nie jak arogancki zrzędliwy (tj. Nie jak ja).
  2. Nie możesz wygrać, możesz tylko stracić mniej.
  3. Skoncentruj się na dodawaniu prawdziwej wartości - nie tylko piękna. Lubię mój kod, aby ładnie wyglądać, ale czasami muszę zaakceptować to, że niedrogie sprawy są ważniejsze.
  4. Dobrym sposobem na znalezienie ulubionego miejsca jest przestrzeganie zasady skautów - kiedy pracujesz nad obszarem kodu, zawsze pozostawiaj go w lepszej formie, niż go znalazłeś.
  5. Mała poprawa jest lepsza niż brak poprawy.
  6. Korzystaj z automatycznych narzędzi. Na przykład narzędzia, które usuwają trochę formatowania, mogą zmienić świat .
  7. Sprzedawaj inne pomysły, które okazjonalnie poprawiają przejrzystość kodu. Na przykład testy jednostkowe zachęcają do rozkładania dużych metod na mniejsze.
Kramii
źródło
5
+1 za korzystanie z automatycznych narzędzi. Szokujące wydaje się, że wielu sklepom nie zależy na tyle, by zobaczyć, jak wygląda zestaw narzędzi dla programistów. Tylko dlatego, że masz kontrolę źródła, edytor i kompilator, nie uzupełnia twojego zestawu narzędzi.
Spencer Rathbun,
4
@Spencer: Nie mogłem się więcej zgodzić. Jednocześnie denerwują mnie programiści, którzy nie używają narzędzi, które mają - z powodu nieznajomości funkcji lub korzyści lub zwykłego lenistwa. Większość współczesnych IDE ma wbudowaną funkcję formatowania kodu, która wymaga tylko kilku naciśnięć klawiszy - ale niektórzy deweloperzy go nie używają.
Kramii
2
Prawdziwe. Ale umieszczam to pod samym sklepem, nie dbając. Twoi deweloperzy mogą nie wiedzieć o niektórych funkcjach w bieżącym zestawie narzędzi, szczególnie jeśli zarządzanie nigdy nie zadało sobie trudu, aby stworzyć standardy. Po drugie, wiele IDE jest bardzo dużych, z ogromnym zestawem funkcji. Używam vima od kilku lat i nadal nie znam wszystkich różnych rzeczy, które mogę z tym zrobić. Jeśli wpuściłeś mnie do Visual Studio, pozostawiłbym 90% funkcjonalności bez zmian, dopóki nie będę miał czasu, żeby to przeszukać. Wtedy może tego nie pamiętam.
Spencer Rathbun,
14

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.

hotpaw2
źródło
Jeśli tam trafiłeś, to procesy, które doprowadziły do ​​tego punktu, prawdopodobnie cię nie zawiodły.
Tim Abell
9

Przegląd kodu służy 3 celom:

  1. Sprawdzanie błędów

  2. Sprawdzanie, gdzie można poprawić kod

  3. 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ń $Avs $Bi 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:

  • BARDZO wyraźnie określ błędy / problemy „należy naprawić” z „To jest najlepsza praktyka i naprawdę powinna zostać naprawiona, JEŚLI możemy zaoszczędzić zasoby - patrz załączona analiza pro / con” ulepszenia projektu (załącz rzeczy opisane w punkcie 2 powyżej) vs „ Są to ogólne wytyczne, które moim zdaniem pomogłyby Ci poprawić odporność kodu, abyś mógł łatwiej utrzymać opcjonalne zmiany w kodzie. Zwróć uwagę na sformułowanie - nie chodzi o „dostosowanie kodu do tego, czego chcę” - to „jeśli to zrobisz, zyskasz korzyści a, b, c”. Ton i podejście mają znaczenie.
DVK
źródło
2
W punkcie 3 recenzje kodu służą nie tylko do nauki autora kodu. Recenzja może być dobrym sposobem na naukę dla mniej doświadczonych programistów, a dla doświadczonych programistów, którzy są nowi w zespole, szybko dostosowują się do standardów kodowania. Omawianie problemów jako grupa może również prowadzić do wglądu w produkt.
Caleb
1
@Caleb - dobra uwaga. Nie chciałem robić zbyt wielu punktów, więc ten został zedytowany z konspektu, ale nadal jest ważny.
DVK
# 4 programistów szkolenia przekrojowego na temat nowych funkcji
Juan Mendes,
1
# 5 - głównym celem przeglądów kodu jest upewnienie się, że dokument projektowy został wdrożony (poprawnie)
Mawg
8

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.

Thanos Papathanasiou
źródło
5
Czy terminy nie zawsze są na horyzoncie?
FreeAsInBeer
8

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ć. :-)

Roger CS Wernersson
źródło
5

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.

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ż:

Hej szefie, byliśmy zgodnie z planem i wszystko działało, ale teraz będziemy odbudowywać wiele rzeczy, abyśmy mogli dodać funkcję X w przyszłości.

lub

Hej szefie, jesteśmy gotowi do wydania. Jeśli kiedykolwiek będziemy musieli dodać funkcję X, może to potrwać kilka dodatkowych dni.

Gdybyś powiedział jedno z nich, twój szef prawdopodobnie powiedziałby:

Kto powiedział coś o funkcji X?

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.

Morgan Herlocker
źródło
Znany również jako YAGNI c2.com/cgi/wiki?YouArentGonnaNeedIt
Juan Mendes
A co powiesz na: „Hej szefie, znasz tę funkcję X, którą chcesz, no cóż, potrzebujemy kilku dni, zanim będziemy mogli zacząć nad nią pracować.”. On też by to chciał. YAGNI nie jest usprawiedliwieniem do tworzenia niechlujnego kodu ani pozostawiania nieuporządkowanego kodu. Trochę długu technicznego nie jest dużym problemem, ale długi muszą zostać spłacone, im szybciej je spłacisz, tym będzie to tańsze.
Thorsal
5

[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.

wprowadź opis zdjęcia tutaj WTF / minutę

Ellen Spertus
źródło
4

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:

Zastanawiam się, czy lepiej byłoby ...

lub

Czy ma sens ...

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ć!

Ed Staub
źródło
1
Rzadko mówię „zastanawiam się, czy lepiej byłoby…”. Powiedziałbym tylko, że gdybym nie był pewien - w takim przypadku autor może pomyśleć, czy byłoby lepiej, czy nie, i może go zmienić, czy nie. Zwykle mówię „Zrobiłbym X”. (Czasami mówię „zrobiłbym X, ale twoje podejście jest lepsze”). Co oznacza, że ​​uważam, że X jest lepszy, ale możesz się nie zgodzić. Albo powiem „to nie działa” lub „to jest niebezpieczne” i lepiej to zmień. Czasami słyszę, że „to nie działa”, i zwykle patrzę na kod, mówię „o cholera”, a potem go naprawiam.
gnasher729,
3

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ść.

Tom Clarkson
źródło
3

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:

  1. Musisz użyć własnego „wpływu” (formy „siły”, która jest pośrednia) i / lub swojej zdolności do „przekonywania”
  2. Zaangażuj się w grupę organizacji zajmującą się „kodowaniem” i zacznij nadawać „konserwacji kodu” wyższy priorytet.
  3. Ugryź pocisk i naucz się czytać gówniany kod szybciej / bardziej płynnie, aby się nie rozłączać (brzmi to tak, jakbyś się rozłączał lub zwalniał, gdy napotykasz gówniany kod) na gównianym kodzie.
    • To sprawi, że będziesz silniejszym programistą.
    • Pozwoli ci to na poprawienie gównianego kodu podczas pracy nad nim.
    • Pozwoli to również na pracę nad większą liczbą projektów, ponieważ wiele projektów ma nieprzyzwoity kod, który jest funkcjonalny ... ale wiele niepotrzebnych kodów.
  4. Dawaj dobry przykład. Ulepsz swój kod ... ale nie staraj się być perfekcjonistą.
    • Ponieważ wtedy staniesz się „powolnym facetem, który nie może dotrzymać terminów, zawsze krytykuje i myśli, że jest lepszy niż wszyscy inni”.

Uważam, że nie ma srebrnej kuli. Musisz użyć wszystkich trzech i musisz być kreatywny w używaniu wszystkich trzech.

Trevor Boyd Smith
źródło
Szkoda, że ​​nie mogę się nauczyć # 3, jestem tak sfrustrowany słabym kodem, że trudno mi nawet go zrozumieć ... ciągle nad tym pracuję ...
Juan Mendes 17.04.13
Projekt jest wadliwy? Jaki projekt
gnasher729,
1

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”.

deworde
źródło
1

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

  1. będzie to optymalne, jeśli zrobimy to w ten sposób.
  2. Pisanie w ten sposób przyspieszy działanie.
  3. Twój kod będzie znacznie bardziej czytelny, jeśli wykonasz „to”, „to” i „to”

Takie uwagi będą traktowane poważnie, nawet jeśli zbliżają się terminy. I prawdopodobnie zostaną wdrożone w następnym cyklu rozwojowym.

Jay D.
źródło
0

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.

kgambrah
źródło
0

Zwiększ jakość recenzji kodu.

Oprócz jakości sprawdzanego kodu istnieje coś takiego jak jakość samego przeglądu kodu:

  • Czy proponowane zmiany są rzeczywiście poprawą ?
  • Czy tylko kwestia opinii?
  • Jeśli recenzent nie wyjaśnił czegoś właściwie, dlaczego ?
  • Jak długo to zajęło? (Widziałem recenzje trwające od miesięcy, a programista jest sprawdzany za rozwiązywanie wszystkich licznych konfliktów scalania).
  • Ton?

Znacznie łatwiej jest zaakceptować dobrej jakości przegląd kodu niż niektóre głównie budzące wątpliwości ego grooming.

h22
źródło
0

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:

  • Uzgodnij standardy kodowania i zasady projektowania.
  • Nigdy nie krytykuj ani nie oceniaj programisty , tylko kod . Unikaj słowa „ty” lub „swój kod”, po prostu porozmawiaj o sprawdzanym kodzie, niepowiązanym z żadnym programistą.
  • Daj się pochwalić jakością ukończonego kodu, aby docenić komentarze, które pomogą poprawić wynik końcowy.
  • Sugeruj ulepszenia zamiast popytu. Jeśli nie zgadzasz się, zawsze miej dialog. Kiedy się nie zgadzasz, spróbuj zrozumieć inny punkt widzenia.
  • Niech recenzje przebiegną w obie strony. Poproszę osobę, którą sprawdziłeś, o sprawdzenie Twojego kodu. Nie mam recenzji „jednokierunkowych”.

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!

JacquesB
źródło