Czasami, kiedy mówimy o jakości kodu, to pojawia się stwierdzenie „nasz kod jest kiepski, bo nie ma w nim komentarzy”. Powstaje taka intuicyjna równoważność: „dużo komentarzy – dobry kod, brak komentarzy – zły kod”. Ba, idąc tą drogą można nawet wprowadzić metrykę, określającą stosunek liczby komentarzy do liczby linii kodu i zmuszać programistów do osiągnięcia jakiejś magicznej jej wartości. Tylko po co? Czy zawsze komentarze ułatwiają nam analizę kodu? Czy istnieją komentarze, które przeszkadzają zamiast pomagać?
Chciałbym pokazać kilka rodzajów komentarzy, które są według mnie złe – czy to ze względu na ich nieprzydatność, źródło pochodzenia czy po prostu „paskudność”.
Szczerze mówiąc, myślałem że na temat jak nie pisać komentarzy powstało już tyle tekstów, że nie warto sobie zawracać tym głowy, ale niestety wujek Google nic na ten temat nie wie. Zaczynajmy więc.
Pierwsza kategoria to komentarze, które mówią co kod robi zamiast tłumaczyć po co to robi. Takie komentarze mogą być przydatne dla kogoś, kto w ogóle nie zna się na programowaniu… ale po co taki ktoś miałby czytać nasz kod? Skąd się biorą? Może z przekonania, że ktoś taki (żona/mąż, kierowniczka/kierownik) będzie chciał ten kod kiedyś przeczytać? Szczerze… – nie liczył bym na to… Według mnie tą kategorię komentarzy (gdybyśmy w amoku kodowanie stwierdzili, że jednak są potrzebne) należy eliminować pisząc zrozumiały kod, precyzyjnie nazywając obiekty, atrybuty, metody…
Lista moich ulubionych komentarzy w tej kategorii poniżej:
[java]
// transform object to object
transform(src, tgt);
[/java]
I już wszystko wiadomo…
[java]
//set value
ourPojoObject.setName(name);
[/java]
Ciekawa sytuacje, kiedy komentarz mówi mniej niż sam kod.
[java]
// return the resulting list
return result;
[/java]
Ilu z Was popełniło kiedyś taki „komentarz”?
[java]
// log info
LogUtils.logDebug(logger, „log = ” + log);
[/java]
Gdyby nazwa metody nie była dość ekspresyjna.
Takie komentarze nie wnoszą nic pozytywnego do naszego kodu. Wnoszą zamęt, niepotrzebnie go „spulchniają” a z biegiem czasu mogą przeobrazić się (jak w ostatnim przykładzie) w kolejną kategorię komentarzy „użytecznych inaczej”.
Do grugiej grupy komentarzy nieprzydatnych zaliczyłbym komentarze jeszcze groźniejsze od poprzednich – komentarze kłamiące. Geneza takich komentarzy może być wieloraka.
Jednym z podstawowych gatunków są komentarze, które z biegiem czasu się „przeterminowały”. Ten gatunek możemy spotkać przeważnie wśród komentarzy metod i klas. Poniżej kilka przykładów z życia wziętych:
[java]
/**
* @param nip – nip number
*
* @return true – if nip is valid, false – if nip is not valid
*/
public void validateNip(String nip) throws ValidationException {
[/java]
Zmiana podejścia do tematu walidacji, zmiana sygnatury metody… zapomnieliśmy „tylko” zmienić komentarz – ktoś się może zdziwić. Pół biedy, jeżeli błąd w opisie jest tak ewidentny, że nawet automat sprawdzający nasz kod się zorientuje. Co jeżeli logika metody jest troszeczkę bardziej skomplikowana, a my stwierdziliśmy, że wygodniej nam będzie, jeżeli zamiast true będziemy zwracać false? Przykład poniżej:
[java]
/**
* Check if there is B52 field in provided dto.
*
* @param dto
* @return true if there is no B52 field in dto
*/
boolean checkFieldB52Exists(Dto dto) {
if (DtoUtilities.getFieldValue(dto, „B52Field”) == null) {
return false;
}
return true;
}
[/java]
Przy okazji warto zwrócić uwagę na to, że lepszym opisem sposobu działania metody jest jej nazwa niż opisujący ją komentarz.
[java]
// log info
LogUtils.logDebug(logger, „log = ” + log);
[/java]
Podobny przykład, tylko trochę mniej groźny.
[java]
} else if („10”.equals(locationId)) { // 10=Back,
// change the action
// action.setTarget(„BACK”);
// action.setPage(currentPage – 1);
} else if („11”.equals(locationId)) {
[/java]
Oczywiście przytoczone przykłady są dość błahe, ale wyobraźmy sobie sytuację, gdzie taki „przeterminowany” komentarz opisuje np. warunki krańcowe działania jakiegoś algorytmu. Jak eliminować takie przypadki? Lekarstwo, jak w wielu przypadkach jest dość proste i w miarę standardowe – przeglądy kodu. Integralną częścią przeglądu kodu powinno być też sprawdzenie czy komentarze są aktualne (i czy w ogóle tam są).
Przy okazji ostatniego przykładu zetknęliśmy się przy okazji z kolejną grupą bezużytecznych komentarzy, a nazywając rzeczy po imieniu – zakomentowanego kodu. Chyba każdy z nas (jeżeli nie, to tylko wypada pogratulować) spotkał się z sytuacją gdzie pojedyncze linijki albo nawet całe metody wiodą swoje drugie życie po życiu zakomentowane – bez żadnego sensu a jedynie z nadzieją, że może kiedyś się przydadzą. Odpowiedzcie sobie sami – czy kiedyś odkomentowaliście taką metodę? Pewnie nie, bo jeżeli nawet, to czy działała według specyfikacji? Przekonanie, że ten kiedyś w bólach wykuty kod, może się nam jeszcze przydać jest widać tak silne, że bardzo często decydujemy się pozostawić taki kod w zakomentowanej formie. A przecież kasując ten kod wcale go nie tracimy – na zawsze pozostanie w naszym systemie kontroli wersji…
[java]
public void transformSequenceToBaModel(SequenceA srcObject, BaModel tgtObject) {
// BaModel seqBaM = tgtObject.createSeqBaM();
// seqBaM.setFirstElement(srcObject.getAElement);
// seqBaM.setSecondElement(srcObject.getBElement);
}
[/java]
Znowu zmuszamy osoby korzystające z naszego kodu do sprawdzenia co metoda tak naprawdę „robi” – zdziwienie może być wielkie…
Z podobnych pobudek – żeby nie zapomnieć, może kiedyś się przyda, lepiej żeby było niż ma nie być – kod przeciętnej aplikacji jest pełen komentarzy, które nazywam komentarzami pochwalno-dupochronowymi:
[java]
// changed by Benek
[/java]
W erze systemów kontroli wersji taki komentarz na prawdę nie ma sensu. Wystarczy w naszym ulubionym IDE z menu wybrać opcję „show annotations” i możemy sprawdzić kto ostatni edytował każdą linię kodu. No chyba, że piszemy takie rzeczy dla zmyłki…
[java]
// asked by email on 20120103
[/java]
Jak mówi mój kierownik – każda zmiana w kodzie, powinna mieć swój ticket w bug trackerze. Tam o wiele dokładnie opiszemy dlaczego i przez kogo została zlecona zmiana.
[java]
// TODO fix that
[/java]
Podobny przypadek jak poprzednio, tylko tym razem wiemy, że coś jest popsute, tylko nie wiemy co i jak używanie takiego kodu może się dla nas skończyć. Zakładając (patrz punkt wyżej), że w naszym bug trackerze mamy ticket opisujący problem w tym miejscu zostawiłbym komentarz TODO, bo zwraca uwagę, że mamy jakiś problem i należy uważać na ten fragment kodu, ale dodałbym referencję do tego ticketu w naszym systemie.
Na koniec coś, co chyba denerwuje najbardziej i przy okazji zajmuje najwięcej miejsca w naszym kodzie – komentarze generowane automatycznie przez IDE. Niby automatyczne generowanie komentarzy powinno nam pomagać. Nie musimy ręcznie wypisywać nazw parametrów, rzucanych wyjątków itp., ale jeżeli nie uzupełnimy szablonów, to kończymy z takimi potworkami:
[java]
/**
* DOCUMENT ME!
*
* @param srcObject
* DOCUMENT ME!
* @param tgtObject
* DOCUMENT ME!
*
* @throws IOXPathException
* DOCUMENT ME!
* @throws ParseException
* DOCUMENT ME!
*/
[/java]
I Eclipse’owy standard – automatyczne nazywanie getter’ów i setter’ów:
[java]
public class VeryImportantClass {
private String evenMoreImportantField;
/**
* @return the evenMoreImportantField
*/
public String getEvenMoreImportantField() {
return evenMoreImportantField;
}
/**
* @param evenMoreImportantField the evenMoreImportantField to set
*/
public void setEvenMoreImportantField(String evenMoreImportantField) {
this.evenMoreImportantField = evenMoreImportantField;
}
}
[/java]
Podsumowując, chciałbym od razu uciszyć głosy krytyki – nie uważam oczywiście, że wszystkie komentarze są złe, ale tak jak ze wszystkim – trzeba zachować umiar. Jeżeli masz pisać złe komentarze – to lepiej nie pisz ich wcale. Oszczędzisz swój czas – ale przede wszystkim oszczędzisz pracy i nerwów osobie, która kiedyś po tobie będzie analizowała i poprawiała kod. Zachęcam także do dodania do listy przeglądu kodu, punktu, którego celem będzie kontrola jakości komentarzy.
I przede wszystkim – nie bierzcie przykładu z przykładów z tego artykułu ;)
Dobry artykuł! Mój faworyt to komentarz w stylu:
# my_method
def my_method
end
Takie to na siłe.
Co do komentarzy kłamiących to pół biedy jeśli taki komentarz kłamie jedynie w temacie parametrów wejściowych. Największe spustoszenie robią komentarze, które opisują coś, a metoda robi coś całkiem innego.
„Fajne” są też kometarze w stylu:
if (foo) {
// this should never happen!
... it tu kilkanaście podchwytliwych linii kodu
}
O tak… komentarze w stylu „to się nie ma prawa stać” są klasyką oddawania projektów na studiach. Najlepiej jeszcze do tego dać jakiś głupkowaty komunikat w interfejsie użytkownika, żeby się profesor miał z czego pośmiać ;)
Zgadzam się z tym, że kod powinien komentować sam siebie. Nie ukrywam, że sam tak pisałem, tutaj jednak należy szukać źródła takich nawyków występującego głównie wśród młodych (chociaż spotkałem się i ze 'starymi’, którzy nie pisali i komentarzy i komentującego samego siebie kodu… ). 'Soczyste’ komentarze dla klas czy metod są dobre tylko w przypadku, gdy chcemy robić jakieś API dla innych, chociaż tutaj też zdarzają się niedociągnięcia i programiście nie będzie się chciało czytać i poprawiać stronnicowego komentarza (błędy i pomyłki zdarzają się nawet w komercyjnych kontrolkach).
Mimo wszystko, najważniejsze, aby nie znalazła się jakaś du*@ w kodzie. :)
Dzis zlapalem sie na tym ze przez kilka minut analizowalem fragment swojego starego kodu, próbując dojsc szczegółów wymagań biznesowych za tym kodem stojacych. Jakież było moje zdziwienie, gdy juz przypomniałem sobie co zas, że wymagania te opisalem w jednej linijce komentarza powyżej kodu. Takie swoiste 'comment blindness’. Wniosek płynie z tego taki: nie pisz komentarzy bo i tak nikt ich nie przeczyta – na czele z Toba – tylko kręcić się będzie w kółko w tańcem bagnistego łabędzia – Paulo Cohelo
Kod powinien być samo-komentujący się. To, że czasem trudno go zrozumieć, to trudno… ;-)
Moje ulubione komentarze:
/* method from hell */
/* do not try this at home */
/* it is fucked, but i don’t have more time */
/* please fix me */