Czy przegląd kodu jest subiektywny czy obiektywny (kwantyfikowalny)?

55

Przygotowuję wytyczne dotyczące recenzji kodu. Nie mamy jeszcze jednego formalnego procesu i próbujemy go sformalizować. Nasz zespół jest rozproszony geograficznie.

Używamy TFS do kontroli źródła (używaliśmy go również do zadań / śledzenia błędów / zarządzania projektami, ale przeprowadziliśmy migrację do JIRA ) z Visual Studio 2008 do programowania.

Jakich rzeczy szukasz podczas przeglądania kodu?

  • To są rzeczy, które wymyśliłem
    1. Egzekwuj reguły FxCop (jesteśmy sklepem Microsoft)
    2. Sprawdź wydajność (jakieś narzędzia?) I bezpieczeństwo (myśląc o użyciu OWASP - przeszukiwacza kodu ) oraz bezpieczeństwo wątków
    3. Przestrzegaj konwencji nazewnictwa
    4. Kod powinien obejmować przypadki brzegowe i warunki brzegowe
    5. Powinien poprawnie obsługiwać wyjątki (nie połykać wyjątków)
    6. Sprawdź, czy funkcjonalność jest powielona w innym miejscu
    7. Ciało metody powinno być małe (20-30 linii), a metody powinny robić jedną rzecz i tylko jedną rzecz (bez efektów ubocznych i unikać czasowego łączenia -)
    8. Nie przekazuj / nie zwracaj wartości null w metodach
    9. Unikaj martwego kodu
    10. Dokumentuj publiczne i chronione metody / właściwości / zmienne

Na jakie inne rzeczy powinniśmy na ogół uważać?

Próbuję sprawdzić, czy potrafimy skwantyfikować proces recenzji (dałby identyczne dane wyjściowe, gdy zostałby sprawdzony przez różne osoby) Przykład: Powiedzenie „treść metody nie powinna być dłuższa niż 20-30 linii kodu” w przeciwieństwie do powiedzenia „metoda ciało powinno być małe ".

Czy też weryfikacja kodu jest bardzo subiektywna (i różni się od jednego recenzenta do drugiego)?

Celem jest posiadanie systemu znakowania (powiedzmy -1 punkt za każde naruszenie reguły FxCop, -2 punkty za nieprzestrzeganie konwencji nazewnictwa, 2 punkty za refaktoryzację itp.), Aby programiści byli bardziej ostrożni, gdy sprawdzali swój kod. W ten sposób możemy zidentyfikować programistów, którzy konsekwentnie piszą dobry / zły kod. Celem jest, aby recenzent spędził maksymalnie 30 minut, aby dokonać przeglądu (wiem, że jest to subiektywne, biorąc pod uwagę fakt, że zestaw zmian / wersja może zawierać wiele plików / ogromne zmiany w istniejącej architekturze itp., Ale otrzymujesz ogólnie rzecz biorąc, recenzent nie powinien spędzać dni na sprawdzaniu czyjegoś kodu).

Jaki inny obiektywny / wymierny system stosuje się do identyfikowania dobrego / złego kodu napisanego przez programistów?

Odnośnik do książki: Clean Code: Podręcznik zwinnego kunsztu programistycznego autorstwa Roberta Martina .

pamięci RAM
źródło
8
Co uważa się za szkodliwe w przypadku zwracania wartości zerowych? Rozumiem, dlaczego zwykle lepiej w językach wysokiego poziomu, takich jak C #, zwracać puste tablice zamiast NULL (sprawia, że ​​kod jest znacznie bardziej elegancki i łatwiej uniknąć błędów). Czasami jednak musisz zwrócić odwołanie NULL, prawda?
4
Jeśli unikniemy zwracania wartości zerowych, możemy pominąć sprawdzanie wartości zerowych, gdy klient / aplikacja / biblioteka wywołująca naszą metodę wywołuje naszą metodę. z Clean Code autorstwa Roberta Martina - Rozdział 7 (Obsługa błędów) pp: 110 „Kiedy zwracamy wartość null, zasadniczo tworzymy pracę dla siebie i stawiamy problemy naszym rozmówcom. Wystarczy jedno brakujące sprawdzenie, aby wysłać rozwijaną aplikację kontroli ”.
3
Czy możesz to wyjaśnić komuś, kto nie chce kupić książki, aby przeczytać jedną stronę :)? Wydaje się, że w przypadku większości programów w języku C # unikanie wartości NULL spowoduje, że kod stanie się bardziej złożony, co z kolei stanowi przepis na więcej błędów ...
2
Oto jeden post na blogu, który wyjaśnia, dlaczego zwracanie wartości null jest złym pomysłem. thehackerchickblog.com/2008/10/… . I jeszcze jeden leedumond.com/blog/should-we-return-null-from-our-methods . Bob sugeruje w swojej książce, że jeśli mamy ochotę zwrócić wartość null, możemy zgłosić wyjątek odniesienia Null lub zamiast tego zwrócić obiekt SPECIAL_CASE. Pomyśl o łączeniu wywołań metod this.Foo().FooBar().FooBarBar(); Jeśli obiekt zwrócony tutaj przez Foo ma wartość NULL, zdecydowanie możesz uniknąć „Odwołanie do obiektu nie ustawione na instancję obiektu”, wywołując FooBar ()
@SoloBold: I tylko dla przypomnienia, to tylko wytyczne. Jeśli istnieje bardzo ważny powód do zwrócenia wartości null (w niektórych przypadkach może być), wówczas zwrócenie wartości null ma sens niż zwrócenie obiektu SPECIAL_CASE

Odpowiedzi:

25

Ocenianie osób w przeglądzie jest sprzeczne z najbardziej udanymi systemami, z którymi pracowałem, a może wszystkim. Ale celem, który staram się osiągnąć od ponad 20 lat, jest zmniejszenie liczby błędów i zwiększenie wydajności na godzinę pracy inżyniera. Jeśli ocenianie osób jest celem, przypuszczam, że można by użyć recenzji. Nigdy nie widziałem sytuacji, w której byłby potrzebny, jako pracownik lub lider.

Niektóre obiektywne badania (Fagan itp.) I wiele popularnych mądrości sugeruje, że relacje rówieśnicze ułatwiają przeglądanie kodu w celu ograniczenia błędów i zwiększenia wydajności. Pracujący kierownicy mogą brać udział jako pracownicy, ale nie jako kierownicy. Podkreślono punkty dyskusji, zmiany w celu zadowolenia recenzentów są na ogół dobrą rzeczą, ale nie są wymagane. Stąd relacja rówieśnicza.

Wszelkie zautomatyzowane narzędzia, które można zaakceptować bez dalszej analizy lub osądu, są dobre - Cint , C ++, Java. Regularna kompilacja. Kompilatory są NAPRAWDĘ dobre w znajdowaniu błędów kompilatora. Dokumentowanie odchyleń w automatycznych czekach brzmi jak subtelne oskarżenie o automatyczne kontrole. Dyrektywy kodowe (podobnie jak Java), które dopuszczają odchylenia, są dość niebezpieczne, IMHO. Idealne do debugowania, aby szybko uzyskać sedno sprawy. Nie tak dobrze jest znaleźć w źle udokumentowanym, 50 000 wierszu kodu bez wiersza komentarza, za który stałeś się odpowiedzialny.

Niektóre zasady są głupie, ale łatwe do egzekwowania; domyślne dla każdej instrukcji switch, nawet jeśli są nieosiągalne, na przykład. To tylko pole wyboru i nie musisz tracić czasu i pieniędzy na testowanie z wartościami, które niczego nie pasują. Jeśli masz zasady , będziesz głupotą , są nierozerwalnie powiązane . Każda korzyść z reguły powinna być warta głupoty, którą to kosztuje, a związek ten powinien być sprawdzany w regularnych odstępach czasu.

Z drugiej strony „biegnie” nie ma cnoty przed przeglądem ani obrony w przeglądzie. Jeśli rozwój postępował zgodnie z modelem wodospadu , chciałbyś dokonać przeglądu, gdy kodowanie zakończy się w 85%, zanim zostaną znalezione i opracowane skomplikowane błędy, ponieważ przegląd jest tańszym sposobem na ich znalezienie. Ponieważ prawdziwe życie nie jest modelem wodospadu, kiedy przeglądanie jest czymś w rodzaju sztuki i jest normą społeczną. Ludzie, którzy faktycznie czytają Twój kod i szukają w nim problemów, są z litego złota. Zarządzanie, które wspiera to na bieżąco, to perła ponad cenę. Recenzje powinny przypominać meldowanie się - wcześnie i często .

Uważam, że te rzeczy są korzystne:

1) Brak wojen stylowych . Tam, gdzie idą otwarte nawiasy klamrowe, należy sprawdzać spójność tylko w danym pliku. Wszystkie takie same. W porządku. Podobnie głębokość wcięcia ** si ** szerokości zakładek . Większość organizacji odkrywa, że ​​potrzebuje wspólnego standardu tab, który jest używany jako duża przestrzeń.

2) `Obdarty

   looking

tekst, który nie ma

   line up is hard to read 

dla treści.`

BTW, K&R wcięły pięć (PIĘĆ) spacji, więc apele do autorytetu są bezwartościowe. Po prostu bądź konsekwentny.

3) Przed przeglądem należy podać numer, niezmienną, publicznie dostępną kopię pliku do przejrzenia na co najmniej 72 godziny.

4) Brak projektowania w locie. Jeśli występuje problem lub problem, zanotuj jego lokalizację i ruszaj się.

5) Testowanie wszystkich ścieżek w środowisku programistycznym jest bardzo, bardzo, bardzo dobrym pomysłem. Testy, które wymagają ogromnych danych zewnętrznych, zasobów sprzętowych, korzystania z witryny klienta itp., To testy, które kosztują fortunę i nie będą dokładne.

6) Format pliku inny niż ASCII jest dopuszczalny, jeśli narzędzia do tworzenia, wyświetlania, edycji itp. Istnieją lub są tworzone na wczesnym etapie rozwoju. To moja osobista stronniczość, ale w świecie, w którym dominujący system operacyjny nie jest w stanie wyjść z siebie z mniej niż 1 gigabajtem pamięci RAM, nie rozumiem, dlaczego pliki mniejsze niż, powiedzmy, 10 megabajtów powinny być czymkolwiek inny niż ASCII lub inny format obsługiwany komercyjnie. Istnieją standardy grafiki, dźwięku, filmów, plików wykonywalnych i towarzyszących im narzędzi. Nie ma usprawiedliwienia dla pliku zawierającego binarną reprezentację pewnej liczby obiektów.

W celu konserwacji, refaktoryzacji lub opracowania zwolnionego kodu jedna grupa współpracowników wykorzystałem recenzję przez jedną inną osobę, siedzącą przy wystawie i patrzącą na różnicę między starym a nowym , jako bramą do odprawy oddziału. Podobało mi się, było tanie, szybkie, stosunkowo łatwe do zrobienia. Wskazówki dla osób, które nie przeczytały wcześniej kodu, mogą być pouczające dla wszystkich, ale rzadko poprawiają kod programisty.

Jeśli jesteś rozproszony geograficznie, patrzenie na różnice na ekranie podczas rozmowy z kimś innym patrząc na to samo byłoby stosunkowo łatwe. Obejmuje to dwie osoby patrzące na zmiany. W przypadku większej grupy, która przeczytała dany kod, wiele witryn nie jest trudniejsze niż wszystkie w jednym pokoju. Wiele pokoi połączonych wspólnymi ekranami komputerowymi i squak-boxami działa bardzo dobrze, IMHO. Im więcej witryn, tym bardziej potrzebne jest zarządzanie spotkaniami. Menedżer jako facylitator może tutaj zarobić na utrzymanie. Pamiętaj, aby nadal odpytywać witryny, w których Cię nie ma.

W pewnym momencie ta sama organizacja przeprowadziła automatyczne testy jednostkowe, które wykorzystano jako testy regresyjne. To było naprawdę miłe. Oczywiście zmieniliśmy wtedy platformy i automatyczny test został pominięty. Recenzja jest lepsza, jak zauważa Manifest Agile , relacje są ważniejsze niż proces lub narzędzia . Ale po przejrzeniu opinii automatyczne testy jednostkowe / testy regresji są kolejną najważniejszą pomocą w tworzeniu dobrego oprogramowania.

Jeśli możesz oprzeć testy na wymaganiach , cóż, jak mówi dama w „When Harry Met Sally” , będę miał to, co ona ma!

Wszystkie recenzje muszą mieć parking, aby uchwycić wymagania i problemy projektowe na poziomie powyżej kodowania. Gdy coś zostanie uznane na parkingu, dyskusja powinna się zakończyć.

Czasami myślę, że przegląd kodu powinien być jak przegląd schematów w projektowaniu sprzętu - całkowicie publiczny, dokładny, samouczek, koniec procesu, brama, po której jest budowany i testowany. Ale przeglądy schematyczne są bardzo ciężkie, ponieważ zmiana fizycznych obiektów jest droga. Przegląd architektury, interfejsu i dokumentacji oprogramowania powinien być prawdopodobnie ciężki. Kod jest bardziej płynny. Przegląd kodu powinien być lżejszy.

Myślę, że pod wieloma względami technologia dotyczy zarówno kultury i oczekiwań, jak i konkretnego narzędzia. Pomyśl o wszystkich improwizacjach „ Swiss Family Robinson ” / Flintstones / McGyver, które zachwycają serce i rzucają wyzwanie umysłowi. Chcemy, żeby nasze rzeczy działały . Nie ma do tego ani jednej ścieżki, podobnie jak „inteligencja”, którą można by jakoś wyabstrahować i zautomatyzować za pomocą programów AI z lat 60 .

Bill IV
źródło
To dobra odpowiedź, szczególnie w odniesieniu do oceniania ludzi - nie powinno to być celem przeglądu kodu.
Paddy
25

Większość opisanych przez ciebie punktów to tylko kwestia formatowania lub „powierzchownych” rzeczy:

  • Przestrzegaj konwencji nazewnictwa
  • Unikaj martwego kodu
  • Dokument
  • ...

Wszystko to można sprawdzić za pomocą zautomatyzowanego narzędzia : nie ma potrzeby, aby doświadczony programista spędzał czas na przeglądaniu kodu, aby na to uważać.

W ogóle nie wiem o .NET , ale dla PHP mamy narzędzia do sprawdzania tego rodzaju rzeczy; biorąc pod uwagę, że .NET często mówi się, że jest „bardziej przemysłowy” niż PHP, zdziwiłbym się, słysząc, że nie ma żadnego narzędzia do sprawdzania tego rodzaju rzeczy.


Zautomatyzowane narzędzie może zarówno:

  • Weź udział w automatycznym procesie kompilacji , który działa co noc
  • Wysyłaj raporty e-mail
    • ostrzeżenia (na przykład metoda jest dłuższa niż 20 linii)
    • błędy (na przykład metoda jest dłuższa niż 50 linii)

Poczta może zostać wysłana do całego zespołu lub do faceta, który popełnił kod, który nie przejdzie testu - lub możesz użyć interfejsu internetowego do raportowania (ta sama uwaga na temat .NET i PHP)


Dodałbym również, że automatyczne testowanie może bardzo pomóc w wykryciu pewnej liczby błędów, zanim kod zostanie użyty w produkcji. Zautomatyzowane testy mogą również pomóc w niektórych pomiarach.


Recenzje kodu wykonane przez doświadczonych programistów mają także kolejną wielką zaletę, o której nie mówiłeś:

  • Doświadczony programista często może wykryć wiele różnych błędów, po prostu patrząc na kod źródłowy (dość często znajduję błędy, gdy robię recenzje kodu)
  • Recenzja kodu wykonana przez doświadczonego programistę pozwoli mu na komentarze i rekomendacje dla zespołu :
    • Spróbuje zrozumieć algorytmy stosowane w kodzie i prawdopodobnie zaproponuje lepsze rozwiązania.
    • Po przeczytaniu kodu często widać rzeczy, których automatyczne narzędzie nie wykryje.

Ale do przeglądu kodu, który idzie głębiej niż tylko formatowanie kodu, potrzebujesz więcej niż pół godziny ...

Pascal MARTIN
źródło
To narzędzie dla .Net (teraz tylko C #) to StyleCop. code.msdn.microsoft.com/sourceanalysis
Bryan Anderson
15

Z moich doświadczeń związanych z przeglądaniem kodu wynika, że ​​powinien to być wspólny wysiłek na rzecz ulepszenia kodu, a nie „środek” decydujący o tym, kto jest dobry czy zły w swojej pracy. Jeśli nie ma znaczenia, czy podczas sprawdzania kodu pojawi się wiele uwag, recenzenci będą bardziej rygorystyczni, dlatego będą sugerować ulepszenia kodu.

Aby poprawić jakość sprawdzanego kodu, wymuszaj przetwarzanie komentarzy recenzji (pozwól recenzentowi zatwierdzić przetworzone komentarze), a także użyj narzędzi do sprawdzania kodu statycznego, aby wymusić poziom jakości dla początkowego zatwierdzenia.

Thirler
źródło
2
+1 za komentarz dotyczący niedopuszczenia, aby stało się to porównaniem tego, kto jest lepszy w swojej pracy. Byłoby to szkodliwe dla morale!
2
@KarstenF: True. Również DeveloperA może pracować z bardziej złożonym zadaniem (więcej wierszy kodu), podczas gdy DeveloperB może pracować z prostym zadaniem i może otrzymać mniej punktów (w skali punktowej). Niesprawiedliwie byłoby powiedzieć, że DevA wykonała złą robotę, gdy nie ma sposobu na normalizację zarówno ich zadań / zadań
2
Również niektórzy programiści mogą próbować zdyskredytować swoich kolegów.
ten punkt jest słuszny. Drobne pojęcia (takie jak ocenianie) prowadzą do małostkowości.
Dan Rosenstark,
+1 w tym bardzo ważnym punkcie. Gdy tylko Twój proces zacznie generować liczbę, ludzie będą grać w swój kod, aby zwiększyć liczbę. Piszą na przykład wiele wierszy prostego kodu, dzięki czemu ich kary / ocena metody są bardzo niskie. Lub spędzają cały swój czas na szukaniu idealnych nazw zmiennych. A potem staje się to kwestią polityczną, ponieważ nikt nie będzie chciał wskazać drobnych błędów w kodzie swojego przyjaciela, ponieważ OBNIŻA SIĘ ich WYNIKI I POWODUJE SIĘ ZŁE! Och nie! Krótko mówiąc, twoje serce jest we właściwym miejscu, ale zły pomysł. Programiści nie są wystawowymi psami.
leoger
5

Myślę, że twój system oceniania to zły pomysł. O co chodzi? Aby zidentyfikować dobrych i złych programistów? Każdy w tym przeglądzie kodu może lepiej ocenić niektóre programiści na podstawie kodu przedstawionego w przeglądzie kodu niż dowolne przypisanie wartości do nieco arbitralnego zestawu cech. Jeśli chcesz zidentyfikować dobrych i złych programistów ... zapytaj programistów. Gwarantuję, że ludzie mogą dokonać tej oceny lepiej niż twoja głupia heurystyka.

Moją propozycją byłoby ulepszenie recenzji kodu, aby ludzie otwarcie dzielili się pomysłami i opiniami w środowisku pozbawionym osądów i wrogim. Jeśli możesz to zrobić, będziesz 100 razy lepszy niż ocenianie programistów na podstawie twoich głupich list kontrolnych, które mają na celu dobrą ocenę programistów. Myślę, że wielu programistów jest już z siebie dumnych i trudnych, jeśli źle radzą sobie z recenzjami kodu; Zastanawiam się, czy dalsza „kara” za słabe wyniki jest ogólnie pomocna.

robarson
źródło
4

Moją jedyną radą byłoby uniknięcie zbyt rygorystycznego procesu sprawdzania kodu - najważniejsze jest to, że przegląd kodu rzeczywiście się dzieje i jest traktowany poważnie .

Im bardziej wyczerpujący jest proces dla recenzenta, tym mniej prawdopodobne jest, że recenzje kodu będą miały miejsce i że będą one traktowane poważnie, a nie tylko jako uciążliwość. Poza tym prawdziwa wartość recenzji kodu polega na tym, że recenzent może użyć własnego osądu, za pomocą zautomatyzowanych narzędzi można sprawdzić takie rzeczy, jak to, czy reguły FXCop miną.

Justin
źródło
+100! Mam na myśli +1, ale tak naprawdę nie o to chodzi: w przypadku recenzji kodu i testów jednostkowych (i innych rzeczy) mniej znaczy więcej. Jest to prawdą tylko dlatego, że więcej znaczy więcej, aż do zera :)
Dan Rosenstark
4

Zasadniczo unikaj spędzania czasu na przeglądaniu kodu, robiąc coś, co można zrobić za pomocą komputera. Na przykład, Twoim pierwszym celem jest „egzekwowanie zasad FxCop”, ale przypuszczalnie może to zrobić FxCop bez konieczności, aby ludzie to robili.

Greg Hewgill
źródło
3

Jeśli możesz to zmierzyć, jeśli jest to obiektywne, kwantyfikowalne, spróbuj użyć narzędzia. Tam, gdzie potrzebujesz doświadczonego recenzenta, to rozmyte subiektywne rzeczy.

kiwicptn
źródło
100 godzin na wykonanie narzędzia, 1000 zapisanych za jego pomocą.
Dan Rosenstark,
3

Wiele dobrych komentarzy zostało już napisanych na temat stylu, co jest ważne. W projekcie zespołowym warto, aby cały kod wyglądał tak, jakby został napisany przez jednego autora. Ułatwia to innym członkom zespołu wpadanie i naprawianie problemów, gdy się pojawią. Jakie środki ilościowe wybierzesz, aby zapewnić ten szerszy cel, są mniej ważne.

Dodatkowym elementem jest zapewnienie zgodności kodu z ogólną architekturą uzgodnioną dla reszty systemu. Podobne problemy należy rozwiązać w ten sam sposób. Jeśli logika aplikacji została podzielona na warstwy, czy sprawdzany kod rozbija jego funkcjonalność tak, jak robi to reszta systemu? Alternatywnie, czy testowany kod uczy czegoś nowego, co powinno zostać wycofane w pozostałej części systemu? Podobnie jak kontrole stylu zapewniają, że kod wygląda tak samo, przegląd architektury powinien zapewnić, że kod działa w ten sam sposób. Ponownie nacisk kładziony jest na łatwość konserwacji. Każdy członek zespołu powinien mieć możliwość zapoznania się z tym kodem i natychmiastowego zapoznania się z tym, co się dzieje.

Pomysł oceniania wydaje się katastrofą, ale znasz swój zespół najlepiej. Możliwe, że będą motywowani przez taki system, ale myślę, że bardziej prawdopodobne jest, że ludzie zaczną się bardziej martwić o swoją ocenę niż rozwiązywanie problemów. Jednym z naprawdę cennych efektów ubocznych przeglądów kodu są oferowane przez nich możliwości mentoringu. Recenzent powinien traktować osobę, która napisała kod, jako osobę, której mentoruje. Każdy znaleziony problem nie stanowi problemu, ale jest szansą na stworzenie bardziej kompetentnego i wyrafinowanego członka zespołu oraz ściślejszego zespołu.

Jim Blanchard
źródło
2

Szczerze mówiąc, bardziej obchodzą mnie rzeczy „subiektywne” niż cokolwiek innego. Po dobrej recenzji kodu chcę, aby ktoś sprawdził moją logikę, a nie pisanie. I na tym się skupiam, kiedy recenzuję kod.

Ogólny format, który lubię, to:

  1. Co naprawiamy?
  2. Co było tego przyczyną? (spójrz na kod)
  3. Jak to naprawiamy?
  4. Pokaż mi nowy kod
  5. Pokaż mi działający kod

Bez tego samo patrzenie na różnice zwykle daje wkład w drobne problemy lub punkty stylistyczne. Bardziej martwi mnie to, czy logika jest poprawna, ogólnie przyjęte podejście jest dobre i czy rozwiązanie będzie możliwe do utrzymania.

Jako przykład ostatnio spojrzałem na jakiś kod współpracownika. Pierwotny problem dotyczył naruszenia FxCop. Ale próbowano ustalić obecność lub brak funkcji systemu Windows, sprawdzając numer wersji. Mój główny wkład polegał na tym, że był to kruchy sposób, aby to zrobić, i lepiej byłoby odpytywać usługę bezpośrednio, ponieważ mapowanie między istnieniem funkcji a SKU systemu Windows może się zmienić w przyszłości i wcale nie jest przyszłościowe.

Kyoryu
źródło
Nie jest jasne z twojej odpowiedzi: czy FxCop złapał tę kruchość, czy ty?
Dan Rosenstark,
2

Cyklomatyczna złożoność (CC) jest jednym ze sposobów oceny kodu, który „nie jest zły”.

W rzeczywistym kodzie, który ma wysokie CC, mam wysoki współczynnik „co tu się dzieje, nie pamiętam”. Niższy kod CC jest łatwiejszy do zrozumienia.

Oczywiście, zwykłe zastrzeżenia dotyczą metryk.

Paul Nathan
źródło
1
@AfermeraInfo: co?
Paul Nathan
1

Przeglądy kodu są zarówno subiektywne, jak i obiektywne. Reguły takie jak „treść metody musi wynosić 20-30 linii” są subiektywne (niektóre osoby mogą myśleć, że 100 linii jest w porządku), ale jeśli Twoja firma zdecydowała, że ​​limit 20-30 linii jest w porządku, możesz to zmierzyć. Myślę, że wymyślony przez ciebie system punktowy to świetny pomysł. Będziesz musiał okresowo dokonywać ponownej oceny, ponieważ okaże się, że niektóre reguły muszą mieć mniejszą lub większą wagę w punktacji, ale dopóki wszyscy znają reguły, wygląda to na dobry system.

Inne rzeczy, których bym szukał:

  • czystość kodu - czasami fragment kodu może być napisany w jednym wierszu lub kilku wierszach. Przeciętny programista nie powinien spędzać kilku minut próbując dowiedzieć się, co robi wiersz kodu. Jeśli tak, może kod powinien zostać przepisany w prostszy sposób. Jest to subiektywne, ale kluczowe jest to, że kod powinien być natychmiast zrozumiały dla większości programistów w Twojej firmie.
  • sprawdź parametry wejściowe funkcji - powinien być jakiś kod, aby sprawdzić, czy parametry wejściowe mieszczą się w dopuszczalnych zakresach. To również powinno pasować do dokumentacji funkcji.
  • opisowe nazwy zmiennych - z wyjątkiem kilku szczególnych przypadków (indeksy pętli itp.), nazwy zmiennych powinny być opisowe. Jednym z zasobów, który możesz chcieć przyjrzeć się konwencjom nazewnictwa itp., Jest Kod Kompletny
TLiebe
źródło
1

Wygląda na to, że zbyt szybko stajesz się zbyt szczegółowy. Powinieneś to bardziej rozbić. Należy przestrzegać kodu pod kątem jego jakości i zgodności funkcji. Powinieneś się rozdzielić i to nawet nie koniec historii ... oto moja propozycja:

Jakość kodu:

  • Zautomatyzowane kontrole:
    • Zgodność stylu: czy konwencja nazewnictwa jest poprawna, czy wszystkie kody są odpowiednio wcięte itp.
    • Standard wydajności: kontrola wycieków pamięci, kontrola złożoności, zmienne redundantne itp.
  • Rzeczywista wzajemna ocena:
    • Prosty spacer po projekcie
    • wyjaśnienie odstępstw od automatycznych kontroli
    • Łatwość konserwacji, porozmawiaj o tym, jak możesz ją utrzymać i wszystko
    • Testowalność: jak łatwo jest przetestować ten kod? Masz plan?

Zgodność funkcji:

  1. Przegląd wymagań funkcji i wszelkich zmian od czasu przeglądu wymagań i / lub projektu
  2. Demonstruj funkcjonalność związaną z wymaganiami i sprawdzaj je pojedynczo
  3. Omów wszelkie dodatkowe wymagania dotyczące innych aspektów oprogramowania napotkanych podczas wdrażania (takich jak plany wdrożenia, infrastruktura itp.)
  4. Wyjaśnienie wszelkich odchyleń od wymagań, jeżeli w tym momencie.

Jeśli potrafisz opisać dwa aspekty przeglądu kodu, jesteś złoty.

jghost
źródło
1

To zależy.

Niektóre części recenzji można łatwo obliczyć (bez problemów z FxCop, bez błędów StyleCop , bez błędów CAT.NET itp.)

Styl może być jednak subiektywny - ale jak mówisz, kiedy zaczniesz być bardziej szczegółowy (bez metody> 20 linii), możesz go zmierzyć, a narzędzia takie jak NDepend mogą to zrobić automatycznie. Niektóre rzeczy nigdy nie będą automatyczne - sprawdzanie obsługi przypadków na krawędzi wymagałoby testów, co powoduje zwiększenie zasięgu kodu, a 100% jest nieosiągalnym ideałem w wielu przypadkach. Sprawdzanie duplikacji jest trudne do wykonania automatycznie. Czeki zerowe, cóż, nie jestem pewien, czy się z tobą zgadzam, ale możesz być w stanie napisać reguły NDepend lub reguły FxCop dla tego.

Im więcej narzędzi, tym lepiej, a jeśli narzędzia pozwalają programistom sprawdzić swoją pracę przed zatwierdzeniem zmian i wykonać kontrole w ramach procesu CI , to zminimalizujesz potrzebę recenzji.

blowdart
źródło
0

System znakowania wydaje się trudny do poprawnego wykonania, ale warto go mieć jako narzędzie pomiarowe: nie można poprawić tego, czego nie można zmierzyć. Ale prawdopodobnie powinieneś zaakceptować fakt, że pewne rzeczy będą trudne / niemożliwe do dokładnego oszacowania. Trudne będzie ustalenie, ile punktów powinna uzyskać każda jakość: na przykład, jeśli przestrzeganie konwencji nazewnictwa daje 2 punkty, to ile punktów za utrzymanie metod na niskim poziomie?

Być może coś w rodzaju prostej listy kontrolnej byłoby lepsze, aby kod mógł zostać oznaczony jako zgodny, częściowo zgodny lub niezgodny z określoną jakością. Później możesz dodać ocenę do listy kontrolnej, gdy zobaczysz, jakie problemy z jakością pojawiają się najczęściej lub powodują najwięcej problemów.

Proces przeglądu powinien być również na tyle elastyczny, aby kod mógł zawieść w części przeglądu, pod warunkiem że można to uzasadnić i udokumentować. Ślepe trzymanie się jakiegoś standardu jakości kodu, który powoduje, że komponent staje się niepotrzebnie skomplikowany / niemożliwy do zarządzania, jest złym pomysłem!

KarstenF
źródło
prefekt dzieje się.
0

Jeśli chcesz, aby kod ludzi był bardziej znormalizowany, nie zmuszając go do „marnowania czasu na formatowanie”, na co narzekają niektórzy programiści. Zainwestuj w narzędzie takie jak ReSharper, ponieważ sprawia, że ​​naprawianie formatowania i innych zadań związanych z faktoringiem jest prawie zautomatyzowanym procesem.

Peter Mortensen
źródło
0
  • Jeśli maszyna może to sprawdzić, ludzie nie powinni.
  • Tylko jeden element listy kontrolnej: Czy każdy przypadek błędu jest obsługiwany poprawnie wszędzie?
  • Użyj recenzji kodu, aby poprawić jakość i przenieść wiedzę.
  • Nie używaj recenzji kodu do identyfikowania „złych” programistów.
  • Ego Dings są bardziej skuteczne niż wyraźne punkty.
  • Krótko mówiąc - 90 minut i 500 linii jest OGROMNA.
najdziwniejszy
źródło