Czy dobrze jest przechowywać komentarze dotyczące błędów w kodzie?

15

Mój zespół używa jasnego przypadku jako kontroli wersji. Projekt, nad którym pracuję, nie rozpoczął się 7-8 lat temu. Przez cały czas trwania projektu mieliśmy kilka wydań poprawek błędów Service Pack itp. Problemy są śledzone za pomocą systemu śledzenia błędów, a większość osób, które pracują nad poprawkami błędów, postępuje zgodnie z procedurą dołączania komentarza do START / Blok END z datą, autorem, identyfikatorem błędu itp.

Wydaje mi się, że jest to zupełnie nieistotne i sprawia, że ​​kod jest zagracony i trudny do utrzymania, i to są rzeczy, które muszą być częścią komentarzy / etykiet odpraw itp., W których możemy przechowywać dodatkowe informacje o cyklu życia produktu pracy.

Jaka jest najlepsza praktyka, której należy przestrzegać?

Niektórzy recenzenci kodu nalegają na komentarze na temat błędu i poprawki, aby ułatwić im życie. W moim rozumieniu muszą przejrzeć pliki, mapując je do widoku i uzyskać dziennik zmian gałęzi i przejrzeć go. Przydałoby się kilka sprawdzonych metod przesyłania zaktualizowanego kodu do przeglądu.

sarat
źródło
3
Prawdziwe pytanie brzmi: DLACZEGO tak to robią. To może być bardzo stara procedura sprzed kontroli źródła.
@ anderson - Wydaje mi się, że nie rozumieją dobrze korzystania z clearcase, kiedy zostało wprowadzone. Aby ta praktyka mogła
pójść
zastanawiałeś się nad tym?
Nie powiem, że to zła praktyka. W każdym razie jednym z pomiarów jakości oprogramowania jest procent komentarzy w kodzie źródłowym.
Rudy,

Odpowiedzi:

27

Problem z dodaniem poprawki jako komentarza do kodu polega na tym, że nie otrzymujesz pełnej historii. Jeśli zobaczę idealnie dobry fragment kodu oznaczony jako „jest to poprawka do bla bla ”, moją pierwszą reakcją byłoby powiedzenie „co z tego?”. Kod jest tam, działa. Jedyne, co muszę wiedzieć, aby zachować kod, to komentarz, który mówi mi, co robi.

Lepszą praktyką byłoby dodawanie odwołań do błędów w dziennikach zatwierdzania SCM. W ten sposób zobaczysz, czym jest błąd, gdzie został wprowadzony i jak został naprawiony. Ponadto, kiedy nadejdzie czas wydania, możesz po prostu wyodrębnić dzienniki SCM i dodać punktor informujący, że wystąpił błąd i został on naprawiony. Jeśli inna gałąź lub wersja wprowadza ten sam błąd, łatwo jest zlokalizować poprawkę i zastosować ją ponownie, jeśli rzeczywiście jest to ta sama rzecz.

Powiedziawszy to wszystko, zgadzam się również z odpowiedzią Charlesa. Jeśli przyczyna kodu nie jest oczywista, należy poinformować opiekuna, że ​​kod istnieje z jakiegoś powodu i należy go traktować ostrożnie.

Dysaster
źródło
Doskonała odpowiedź. Dodałem jeszcze kilka punktów do mojego pytania. Sprawdź i zaktualizuj swoją odpowiedź. Dziękuję Ci. BTW, czy mógłbyś mi pomóc z komendami clearcase, aby uzyskać dzienniki zatwierdzeń z określonej gałęzi?
sarat
@Sarath Obawiam się, że nigdy wcześniej nie korzystałem z ClearCase. Zapraszam do skorzystania z superużytkownika. Jestem pewien, że jest wielu ludzi chętnych do pomocy.
Dysaster
4
Dobre narzędzia do śledzenia błędów, takie jak JIRA, mogą przeglądać dzienniki zatwierdzeń w SCM i wyciągać informacje o błędach. Pomyśl tylko, że popełniasz błąd w związku z błędem, a moduł śledzenia błędów automatycznie dodaje notatkę do raportu o błędzie, który potwierdza, że ​​X odnosi się do błędu.
@Andersen - Dziękujemy, korzystamy z JIRA. Ale muszę się upewnić, czy używa poprawnie, czy nie.
sarat
@ Thorbjørn Ach, masz to łatwe. Musiałem to zrobić ręcznie w ciągu dnia za pomocą kombinacji CVS / Bugzilla (a później SVN / Bugzilla). Dodaj odwołanie do poprawki do mojego zatwierdzenia i dodaj odwołanie do bugzilli. Proces podatny na błędy, a programiści zwykle zapominają o jednym lub drugim. Ale informacje te były bardzo przydatne przy wielu okazjach.
Dysaster
23

Głównie zła praktyka. Nie powiem, że nigdy nie należy tego robić. Czasami napotykasz na błąd w zewnętrznym interfejsie API, który musisz obejść. Obejście problemu może wyglądać na zupełnie martwe, jeśli nie wiesz o podstawowej usterce. W takim przypadku dobrym pomysłem może być udokumentowanie błędu w kodzie, aby współpracownicy lub późniejsze osoby nie próbowały „naprawiać” oczywistego martwego kodu.

Charles E. Grant
źródło
3
Zgoda. Dodaj tego rodzaju komentarze, gdy dodają znaczną wartość. Nie rób ich jednak tylko po to, aby przekręcić uchwyt.
szybko_now
Fajnie, to wspiera pomysł komentowania „Dlaczego” jakiegoś kodu. Zobacz programmers.stackexchange.com/questions/1/…
Gabriel,
Zrobiłem to już kilka razy: kod, który na pierwszy rzut oka całkowicie przeciwstawia się logice („po co ten kurwa tutaj jest ten kod?”), Ale tak naprawdę istnieje z bardzo dobrego powodu, więc chcesz, aby ludzie obchodzili się ostrożnie to. Następnie dodanie komentarza wyjaśniającego, dlaczego ten kod może ułatwić życie wszystkim. Potem, jeśli ktoś może wymyślić lepszy sposób na rozwiązanie pierwotnego problemu, powiadam, że to wszystko dzięki, że wie, dlaczego kod braindead został wprowadzony i co powinien osiągnąć, więc o wiele łatwiej jest zaimplementować alternatywę rozwiązanie.
CVn
9

Zła praktyka. Komentarze będą nieaktualne i zaśmiecą kod. W razie potrzeby informacje są nadal dostępne w historii wersji systemu SCC.

Craig
źródło
3

Brzmi jak zła praktyka. Co jeśli Twoja organizacja zdecyduje się na migrację do innego systemu śledzenia błędów? Nie przywiązuj produktu zbyt mocno do aktualnie używanych narzędzi. Zamiast odwoływać się do konkretnych identyfikatorów błędów, a przyczyny, dla których kod wygląda tak, jak jest, są niejasne, uzasadnij swoją decyzję projektową komentarzami w kodzie.

fejd
źródło
To jest fałszywa dychotomia. Dlaczego nie możesz mieć komentarzy projektowych („dlatego zrobiliśmy xy z”), gdy jest to konieczne, i podawać identyfikatory błędów w komunikatach zatwierdzania?
Matthew Flaschen
Gdzie to mówi, że nie możesz? Miałem na myśli identyfikatory błędów w kodzie, a nie komunikaty zatwierdzania VCS.
fejd
Przepraszam, źle zrozumiałem. Myślałem, że mówisz, że nigdzie nie warto umieszczać identyfikatorów błędów.
Matthew Flaschen
Nie ma problemu, to tylko oznacza, że ​​moja odpowiedź nie była wystarczająco jasna. :)
fejd
2

Moją pierwszą reakcją byłoby nie powtarzanie się, więc wyjmij to z kodu i do dzienników SCM. Przeprowadziliśmy podobną dyskusję tutaj na temat komentarza do rewizji funkcji, nazwisk autorów oraz dat utworzenia plików i funkcji. W przeszłości (przed użyciem SCM) wszystkie te informacje były przechowywane w plikach, aby móc odtworzyć ewolucję pliku.

Około połowa programistów chce, aby te informacje mogły mieć wszystkie informacje w jednym miejscu (powoduje to, że szukają zmian w SCM). Druga połowa programistów nie rozpoczyna poszukiwań wskazówek co do zmian w kodzie, ale z SCM, więc nie potrzebują informacji w kodzie. Musimy jeszcze podjąć decyzję, co zrobić z tymi komentarzami. To bardzo zależy od sposobu, w jaki ludzie pracują, a niektórzy ludzie są bardzo uparci, aby porzucić swoje znane metody. To samo dotyczy komentowania bloku kodu i pozostawiania ich w kodzie na zawsze.

refro
źródło
Myślę, że skomentowany kod powinien zostać sprawdzony przez SCM i odmówił nawet popełnienia ... To dodaje bałaganu do kodu tylko po to, aby zyskać 5 minut wyszukiwania w historii SCM, jeśli jakiś hipotetyczny dzień w przyszłości będzie potrzebny. plecy.
Julien Roncaglia
Zgodzono się, ale spróbuj zaimplementować to w kodzie źródłowym: D W naszym zespole projektowym pracujemy z recenzjami, więc na razie recenzentowi odmawia tych bloków.
refro
Oh SourceSafe ... przepraszam za ciebie i twój zespół.
Julien Roncaglia
Nie bądź, to lepsze niż nic. W tej chwili wszystkie nowe prace rozwojowe są wykonywane w Subversion, więc z każdym miesiącem mam mniej do czynienia z bezpieczeństwem źródeł.
refro
1

Żeby dodać do tego, co Dyaster i in. powiedziałem, chociaż JIRA ma naprawdę fajne możliwości wyświetlania zmian związanych z poprawkami błędów, absolutnie najlepszym miejscem do udokumentowania poprawki jest przypadek testowy. Jeśli kod nie jest jasny bez komentarza wskazującego, który błąd został naprawiony, oznacza to „zapach kodu”. To powiedziawszy, jeśli nie masz czasu na usunięcie zapachu, komentarz powinien odnosić się do przypadku testowego, gdzie powinno być o wiele bardziej oczywiste, dlaczego kod robi to, co robi. Jeśli nie masz czasu na napisanie instrukcji testowej wyjaśniającej naprawę błędu, oznacza to, że błąd nie został jeszcze naprawiony, został tylko odłożony.

Ben Hocking
źródło
1

Zgodziłbym się na dodanie identyfikatorów błędów w komunikatach zatwierdzania, a nie w samym kodzie. Bardzo przydatne są narzędzia do śledzenia błędów, które automatycznie zgarniają komunikaty zatwierdzania dla identyfikatorów błędów.

Ponadto można użyć polecenia blame / annotate / praise w systemie kontroli wersji, aby zastąpić te komentarze. Następnie, gdy uruchomisz coś takiego:

vcs blame file.ext

możesz zobaczyć przydatne informacje, zwykle w tym kto zmienił każdą linię, kiedy ją zmienił, oraz identyfikator zatwierdzenia. Z identyfikatora zatwierdzenia można uzyskać pełną wiadomość, która powinna zawierać identyfikator błędu.

Dobre systemy VCS pozwolą ci zignorować białe znaki podczas obliczania, kto zmodyfikował linię.

Nie wiem, co ma Clear Case dla tej funkcji.

Matthew Flaschen
źródło