Konserwacja kodu: Aby dodać komentarze do kodu lub po prostu pozostawić kontrolę wersji?

42

Zostaliśmy poproszeni o dodanie komentarzy do tagów początkowych, końcowych, opisu, rozwiązania itp. Dla każdej zmiany, którą wprowadzamy do kodu w ramach naprawy błędu / implementacji CR.

Obawiam się, czy zapewnia to jakąś wartość dodaną? W tej chwili mamy wszystkie szczegóły w historii kontroli wersji, które pomogą nam śledzić każdą zmianę?

Ale moi przewodnicy nalegają, aby komentarze były „dobrą” praktyką programistyczną. Jednym z ich argumentów jest to, że CR należy zmniejszyć / zmienić, byłoby kłopotliwe, gdyby nie było komentarzy.

Biorąc pod uwagę, że zmiany byłyby w dużej mierze pomiędzy kodami, czy naprawdę pomogłoby dodanie komentarzy do każdej wprowadzanej przez nas zmiany? Czy nie powinniśmy pozostawić kontroli wersji?

Chillax
źródło

Odpowiedzi:

43

Masz absolutną rację. Śledzenie zmian to zadanie twojego systemu kontroli wersji. Za każdym razem, gdy wykonujesz zatwierdzenie, powinieneś napisać komunikat potwierdzający, co zostało zrobione, i odwołujący się do systemu śledzenia błędów, jeśli jest to poprawka. Wstawienie komentarza do kodu mówiącego

// begin fix for bug XXXXX on 10/9/2012
...
// end fix for bug XXXXX

za każdym razem, gdy naprawisz błąd, twój kod będzie szybko nieczytelny i niemożliwy do utrzymania. Spowoduje to również zduplikowanie tych samych informacji w dwóch miejscach, co jeszcze pogorszy bałagan.

Komentarze nie powinny być wykorzystywane do śledzenia błędów, a także nie powinny opisywać tego, co robi Twój kod. Powinny wyjaśnić, dlaczego robisz X lub dlaczego robisz X w ten konkretny sposób. Jeśli czujesz potrzebę napisania komentarza wyjaśniającego, co robi blok kodu, jest to zapach kodu wskazujący, że powinieneś przeformułować ten blok na funkcję o nazwie opisowej.

Więc zamiast

// fixed bug XXXXX on 10/9/2012

możesz mieć komentarz, który mówi

// doing X, because otherwise Y will break.

lub

// doing X, because doing Y is 10 times slower.
Dima
źródło
12
+1 za kod pachnie komentarzami wyjaśniającymi „co”. Miło jest widzieć odpowiedź, że komentarze do kodu nie są automatyczną korzyścią w tym sensie, że więcej komentarzy> mniej komentarzy. Mógłbym nawet cofnąć się o poziom i pomyśleć, że są przypadki, w których nawet komentarze opisujące „dlaczego” mogą być zapachem wskazującym, że kod nie jest jasny. Na przykład, jeśli mogę wstrzyknąć BubbleSorter lub QuickSorter, komentarz „Używam QuickSorter, ponieważ jest szybszy” jest zbędny w taki sam sposób, jak „wstrzyknięcie Quicksorter” jest zbędny. YMMV.
Erik Dietrich,
53

Użyj najlepszego narzędzia do pracy. Twój system kontroli wersji powinien być najlepszym narzędziem do rejestrowania błędów i CR: automatycznie rejestruje datę i osobę, która dokonała zmiany; nigdy nie zapomina dodać wiadomości (jeśli skonfigurowałeś ją tak, aby wymagała wiadomości zatwierdzania); nigdy nie zawiera adnotacji do niewłaściwej linii kodu lub przypadkowo usuwa komentarz. A jeśli twój system kontroli wersji już działa lepiej niż twoje komentarze, niemądrze jest powielać pracę dodając komentarze.

Czytelność kodu źródłowego jest najważniejsza. Baza kodu, która jest zaśmiecona komentarzami, zawierającymi pełną historię każdej poprawki i CR, wcale nie będzie bardzo czytelna.

Ale nie pomijaj komentarzy całkowicie: dobre komentarze (nie niewolnicze dokumentowanie każdego startu / stopu / opisu / rozwiązania każdej poprawki błędu i CR) zwiększają czytelność kodu. Na przykład w przypadku trudnego lub niejasnego fragmentu kodu, który dodajesz w celu naprawienia błędu, komentarz formularza // fix ISSUE#413informujący ludzi, gdzie można znaleźć więcej informacji w narzędziu do śledzenia problemów, jest doskonałym pomysłem.

Josh Kelley
źródło
29
Zgadzam się, z wyjątkiem jednej rzeczy: fix ISSUE#413nie jest dobrym komentarzem w kodzie. Powinieneś być w stanie zrozumieć kod bez konieczności odwoływania się do dokumentacji zewnętrznej. Zamiast podawać losową liczbę, faktycznie wyjaśnij, dlaczego ta podstępna część kodu jest wymagana do robienia tego. Do tego służą komentarze: wyjaśnienie tych części kodu, które nie są oczywiste.
poke
12
@poke - Dziękujemy za zwrócenie na to uwagi. Wydaje mi się, że powinienem dodać, że jedynym miejscem, w którym używam komentarzy w formularzu, fix ISSUE#413jest problem, w którym problem jest tak skomplikowany (skrajnie zależny od systemu operacyjnego i konfiguracji przypadek narożny lub wywołany tylko przez dane złych klientów), że jego odpowiednie opisanie wymagałoby kilka akapitów; tego typu sprawą lepiej zajmuje się moduł do śledzenia problemów IMO. Nawet wtedy krótki opis jest dobry.
Josh Kelley,
8
@poke: Powiedziałbym, że komentarz, który zaczyna się od, fix ISSUE#413 jest całkowicie w porządku, a nawet preferowany, pod warunkiem, że zawiera także rozsądną ilość informacji na temat problemu nr 413. Samo podsumowanie raportu o problemie bez podania wskaźnika utrudnia życie przyszłemu czytelnikowi, który potrzebuje wszystkich szczegółów.
Keith Thompson,
Zgadzam się z poke - nigdy nie powinieneś odwoływać się do zewnętrznego źródła, aby zrozumieć kod. Jeśli sprawdzam zmianę, przerywa to przepływ. Muszę przejść do narzędzia do śledzenia problemów, wyciągnąć problem i przeczytać o nim wszystko. A co się stanie, jeśli zmienisz narzędzie do śledzenia problemów? Może być dobrze mieć fix ISSUE#413w komentarzu kompletność, ale nie używaj go jako kuli.
Michael Dean,
„nigdy nie zapomina o dodaniu wiadomości (jeśli skonfigurowano ją tak, aby wymagała komunikatów zatwierdzania); nigdy nie adnotuje niewłaściwej linii kodu lub przypadkowo usuwa komentarz”. Właśnie poradziliśmy sobie z uszkodzeniem SVN i koniecznością przywrócenia go z kopii zapasowej. Udało nam się znaleźć kod, który nie został jeszcze utworzony do utworzenia kopii zapasowej, ale kiedy ponownie zatwierdziliśmy zmiany, kilka oddzielnych zatwierdzeń staje się jednym. Chodzi mi o to, że nigdy nie jest zbyt mocnym słowem i nie zapominajmy, że ludzie przechodzą na nowe oprogramowanie VCS, a wprowadzenie historii zmian może być niemożliwe lub niemożliwe.
Andy,
7

Komentarze w kodzie dotyczą tego, czym jest kod w danym momencie. Wykonanie migawki w dowolnym momencie nie powinno odnosić się do starych (lub gorszych, przyszłych) wersji kodu.

Komentarze w VCS dotyczą zmian kodu. Powinny one czytać jako opowieść o rozwoju.

Czy każda zmiana powinna zawierać komentarze? w większości przypadków tak. Jedynym wyjątkiem, jaki sobie wyobrażam, jest to, że oczekiwane zachowanie zostało już udokumentowane, ale nie było to, co dostałeś, z powodu błędu. Naprawienie go powoduje, że istniejące komentarze są bardziej precyzyjne, więc nie trzeba ich zmieniać. Sam błąd powinien być udokumentowany w historii zgłoszenia i komentarzu do zatwierdzenia, ale tylko w kodzie, jeśli kod wygląda dziwnie. W takim przypadku // make sure <bad thing> doesn't happenpowinno wystarczyć.

Javier
źródło
8
Głosowałbym za tym, ale tak naprawdę nie mogę zgodzić się z „każda zmiana powinna zawierać komentarze? W większości przypadków tak”. Komentarz do odprawy / zatwierdzenia, tak, absolutnie. Komentarze do kodu, zdecydowanie niekoniecznie.
CVn
6

Jeden rodzaj komentarza, który naprawdę doceniam to:

// To zaimplementowano dla reguły biznesowej 5 wniosku 2

lub cokolwiek do cholery używasz do zebrania swoich wymagań.

Ma to dwie zalety, jedną z nich jest to, że pozwala ci znaleźć powód, dla którego zaimplementowałeś dany algorytm bez wyszukiwania, a drugą jest to, że pomoże ci komunikować się z inżynierami niebędącymi programistami, którzy pracują / tworzą dokumenty wymagań.

Może to nie pomóc w przypadku mniejszych zespołów, ale jeśli masz analityków, którzy opracowują twoje wymagania, może być nieoceniony.

Bill K.
źródło
2
Jest to jednak inne, ponieważ zapewnia możliwość śledzenia prostopadłą do kontroli wersji: połączenie między kodem a implementowaną specyfikacją wymagań.
Kaz
W systemie, w którym kontrola wersji jest połączona z systemem błędów / wymagań, zapewniona jest pełna identyfikowalność bez potrzeby komentowania. Czasami pomocne jest działanie w drugą stronę. Biorąc pod uwagę plik z SCM, pokaż mi, jakie wymagania zostały zaimplementowane, kiedy. Lub, biorąc pod uwagę wymaganie, pokaż mi wszystkie pliki zmodyfikowane w celu jego wdrożenia.
iivel
4

Twoje wskazówki mają rację, gdy mówią, że komentarze są dobrą praktyką programistyczną, jednak są wyjątki. Dodanie komentarza do każdej wprowadzonej zmiany jest jedną z nich. I masz rację mówiąc, że powinno to należeć do systemu kontroli wersji. Jeśli musisz przechowywać te komentarze w jednym miejscu, to VCS jest właściwym rozwiązaniem. Komentarze w kodzie źródłowym mają tendencję do starzenia się i są nieaktualne. Żadne komentarze nie są o wiele lepsze niż złe komentarze. To, czego nie chcesz, to komentarze w obu miejscach (w kodzie i VCS), które nie są zsynchronizowane. Celem jest utrzymanie stanu SUCHOŚCI poprzez posiadanie jednego źródła prawdy dla zmian w kodzie.

marco-fiset
źródło
3

Oprócz tego, co powiedzieli inni, zastanów się, co się stanie, jeśli zmiana wywoła efekt falowania w całym systemie. Załóżmy, że refaktoryzujesz część podstawowego interfejsu w procesie implementacji żądania zmiany - ten rodzaj zmiany może łatwo dotknąć dużego odsetka plików kodu źródłowego w dowolnym nietrywialnym oprogramowaniu, co sprowadza się do trywialnych zmian (klasa lub zmiany nazwy metody). Czy powinieneś przejrzeć każdy plik dotknięty taką operacją, aby ręcznie dodać adnotację do takich komentarzy, zamiast polegać na tym, że VCS robi to wszystko automatycznie? W jednym przypadku patrzysz na nieco więcej niż pięć minut pracy przy użyciu dowolnego przyzwoitego narzędzia do refaktoryzacji, a następnie rekompilacji, aby upewnić się, że nic nie zepsuło się na kompilacji, podczas gdy drugi może łatwo przejść do pracy w ciągu jednego dnia. Dla jakiej konkretnej korzyści?

Zastanów się również, co się dzieje, gdy przenosisz części kodu. Jeden z programistów baz danych, z którymi pracuję, w dużej mierze „każda linia SQL powinna być opatrzona adnotacją o wersji, w której została zmieniona, i przygotujemy osobne historie wersji dla każdego pliku, ponieważ wtedy łatwiej będzie zobaczyć kto zmienił co, kiedy i dlaczego ”. Który działa trochę, jakoś w porządku, gdy zmiana jestw kolejności zmiany pojedynczych linii. Nie działa tak dobrze, gdy, tak jak ostatnio, aby naprawić poważny problem z wydajnością, rozbijasz części większego zapytania wprowadzając tabele tymczasowe, a następnie zmieniasz kolejność niektórych zapytań, aby lepiej dopasować nowy przepływ kodu. To prawda, że ​​różnica w stosunku do poprzedniej wersji była w dużej mierze bez znaczenia, ponieważ informowała, że ​​około dwie trzecie pliku uległo zmianie, ale komentarz dotyczący odprawy był również czymś w rodzaju „poważnej reorganizacji w celu rozwiązania problemów z wydajnością”. Kiedy spojrzałeś ręcznie na dwie wersje, było całkiem jasne, że duże części naprawdę były takie same, tylko się poruszały. (I zajęło omawianej procedurze przechowywanej od regularnego wykonywania ponad pół minuty, do kilku sekund. Do tego czasu

Z nielicznymi wyjątkami śledzenie zmian i odwoływanie się do problemów jest pracą VCS, IMNSHO.

CVn
źródło
3

Zwykle przestrzegam tej zasady: jeśli zmiana jest oczywista, a wynikowy kod nie budzi pytań, nie przywraca ani nie zmienia istotnie żadnego wcześniejszego zachowania w znaczący sposób - pozostaw to VCS śledzenie numerów błędów i innych informacji o zmianie.

Jeśli jednak nastąpi zmiana, która nie jest oczywista, zmienia logikę - szczególnie znacząco zmienia logikę wykonaną przez kogoś innego w nieoczywisty sposób - bardzo korzystne może być dodanie czegoś takiego: „ta zmiana polega na zrobieniu tego i że z powodu błędu # 42742 ". W ten sposób, gdy ktoś patrzy na kod i zastanawia się „dlaczego to jest? Wygląda to dziwnie”, ma przed sobą pewne wskazówki i nie musi przeprowadzać dochodzenia za pośrednictwem VCS. Zapobiega to również sytuacjom, w których ludzie przerywają zmiany innych, ponieważ znają stary stan kodu, ale nie zauważają, że został on zmieniony od tego czasu.

StasM
źródło
2

Komentarze związane z kontrolą wersji nie należą do pliku źródłowego. Dodają tylko bałaganu. Ponieważ prawdopodobnie muszą być umieszczone w tym samym miejscu (jak blok komentarzy na górze pliku), spowodują konflikty uciążliwe tylko dla komentarzy, gdy równoległe gałęzie zostaną połączone.

Wszelkie informacje śledzenia, które można wyciągnąć z kontroli wersji, nie powinny być powielane w treści kodu. Dotyczy to głupich pomysłów, takich jak słowa kluczowe kasowania RCS, takie jak $Log$i podobne .

Jeśli kod kiedykolwiek wykracza poza zakres systemu kontroli wersji, ten ślad komentarzy na temat jego historii traci kontekst, a zatem większą część jego wartości. Aby poprawnie zrozumieć opis zmiany, potrzebujemy dostępu do wersji, abyśmy mogli zobaczyć różnicę do poprzedniej wersji.

Niektóre stare pliki w jądrze Linuksa mają duże bloki komentarzy historii. Te pochodzą z czasów, gdy nie było systemu kontroli wersji, tylko tarballi i łatki.

Kaz
źródło
2

Komentarze w kodzie powinny być minimalne i dokładne. Dodanie informacji o usterce / zmianie nie jest cenne. Powinieneś użyć do tego kontroli wersji. Jakiś czas Kontrola wersji zapewnia nieco lepszy sposób zmiany - używamy ClearCase UCM; Działania UCM są tworzone na podstawie numerów defektów, obszaru zmian itp. (Np. Defect29844_change_sql_to_handle_null).

W komentarzach do odprawy preferowane są szczegółowe komentarze.

Wolę podać szczegółowe informacje na temat podstawowych informacji, szczegóły rozwiązania NIE wdrożonego z powodu niektórych skutków ubocznych.

Pramagic Programmer i CleanCode prowadzi do następujących wskazówek

Zachowaj wiedzę niskiego poziomu w kodzie, do którego on należy, i rezerwuj komentarze dla innych wyjaśnień wysokiego poziomu.

Jayan
źródło