Kilka słów komentarza o… komentarzach

Pod jednym z ostatnich postów wywiązała się krótka dyskusja na temat komentowania kodu i postanowiłem poczynić follow-up w postaci osobnej notki.

Mam cały czas na myśli oczywiście komentarze wyjaśniające działanie kodu a nie zakomentowany kod. Zakomentowany kod powinien być usuwany na poziomie repozytorium w momencie check-ina i problem mielibyśmy z głowy. Na ten temat nawet nie będę się rozwodził.

Komentarze są fe!

Jeśli miałbym w jednym zdaniu zmieścić swój pogląd na komentarze to właśnie tak by ono brzmiało. Powodów jest wiele.

Jeśli skupiamy się na pisaniu komentarzy to nie skupiamy się tak mocno na jakości produkowanego kodu. Po co pisać łatwy w zrozumieniu kod, skoro jest on (w teorii) wytłumaczony obok? Dawno temu, jeszcze na praktykach, “boss” zerknął mi przez ramię i powiedział: “ooo, dużo zielonego, dobrze, ładny kod”. Nawet nie spojrzał na faktyczny kod – zobaczył tylko że 70% ekranu pokrywały komentarze (bo mnie tak uczyli). Jak można ocenić czyjąś pracę na podstawie tego ile napisał komentarzy? To jak ocenić pracę kuriera na podstawie tego ile kilometrów przejechał, a nie ile przesyłek dostarczył. Bez sensu.

Po drugie: gdy jedna osoba pisze komentarz to druga automatycznie jest zobligowana do jego przeczytania. Często nie odpowiada mi nawet zapoznawanie się z nieładnie napisanym kodem, no ale trudno – taki zawód. Ale gdy jeszcze do tego dochodzą wypracowania w postaci komentarzy to mnie krew zalewa. A czytać – czas stracić – muszę, bo jak nie przeczytam to potem będzie że coś napsułem a “przecież jest w komentarzu napisane co i jak!”.

Pół biedy jeszcze, gdy w treści zawarte jest faktycznie to co być powinno. Niestety – nie jest to regułą. Komentarze mają to do siebie, że bardzo lubią rozsynchronizowywać się z tym, co mają opisywać. A bo trzeba było szybko zrobić fix – to się zmieniło kod, ale nie zaktualizowało komentarza. Albo jeszcze w trakcie pracy ta sama dokładnie osoba wpadła na jakiś nowy pomysł i umieściła zmianę tylko w jednym miejscu. Albo ktoś inny przejął kod i metodą prób i błędów doprowadził do pożądanego zachowania, ale istniejącego komentarza przecież nie wyrzuci bo nie jego i nie kazali. A potem kolejny biedny dev głowi się dlaczego na zielono napisane jest jedno, a kod działa inaczej. Mało tego – program działa tak jak wynika z kodu, ale nie tak jak powinien – bo komentarz mówi co innego. Nie skłamię chyba jeśli napiszę, że częściej widywałem komentarze nieaktualne niż aktualne. Powód jest bardzo prosty: komentarz jest aktualny tylko w momencie jego pisania, podczas gdy kod ewoluuje. I zwykle komentarze nie ewoluują razem z nim.

Ale przypuśćmy, że przytrafiła się sytuacja, w której faktycznie 4-5 słów wyjaśnienia by się przydało. I wielkiej krzywdy nikomu tym nie wyrządzono… tym razem. A przecież znana jest wszystkim metoda Copy’ego/Paste’a, gdzie do schowka wędruje wszystko co się akurat zaznaczy w edytorze. Potem ze schowka całość wskakuje w inne miejsce, w potencjalnie inny kontekst. Kod się doszlifuje, dokręci tak żeby było w porządku. A komentarze? Zostają stare. Masz babo placek.

Zielone elaboraty są szczególnie irytujące, gdy ktoś nieznający zbytnio angielskiego sili się na wielce szykowne formy opisania swojej pracy. Osoba po drugiej stronie repozytorium musi wtedy odwrócić proces “polski -> translator -> pseudoangielski”, domyślając się co może oznaczać dane słowo w ogóle niepasujące do kontekstu, a będące jakimś przypadkiem synonimem polskiego wyrazu mającego tutaj zastosowanie. Olaboga, dość.

Nie przemawia do mnie argument, że “komentarze są potrzebne bo kod jest brzydki i zły”. Nie widzę tutaj w ogóle logiki. Skąd założenie, że ktoś, kto pisze “zły i brzydki” kod, napisze “dobre i ładne” komentarze? Jest wręcz przeciwnie: bo przecież komentarz nie wywali błędu kompilacji ani nie wywróci systemu w runtime.

Dochodzimy prawie do końca litanii, ale nie może zabraknąć jeszcze jednego aspektu: komentarzy nadmiarowych. Nawkładano kiedyś komuś do głowy, że kod trzeba komentować. I potem musimy oglądać takie coś:

/// <summary>
/// Class for getting data from database
/// </summary>
public class DataAccess
{
    /// <summary>
    /// Gets user by id
    /// </summary>
    /// <param name="id">User id</param>
    /// <returns>Returns user</returns>
    public User GetUserById(int id)
    {
        if (id < 1)
        {
            // returning null
            return null;
        }
        // getting user from database
        return _database.GetUserById(id);
    }
    // ...

Przejaskrawione? Pewnie tak. Ale narzędzia takie jak GhostDoc właśnie takie coś promują. A same są również nierzadko promowane jako “must-have” w dotnetowym przyborniku. Let this motherfuck*r burn.

Alternatywy

No dobra, ale co zamiast komentarzy? Odpowiedź jest prosta. A właściwie są dwie. Proste.

Po pierwsze: staranne strukturyzowanie kodu. Zamiast komentować kilka linijek – zamknij je w osobną metodę o wiele mówiącej nazwie, człowiecze. Taka metoda będzie pewnie prywatna, więc nie musisz trzymać się CamelCase które przy więcej niż 3 słowach staje się masakrycznie nieczytelne. Pomyśl przez chwilę jak skrócić komentarz do kilku wyrazów i nazwij tak metodę, nawet jeśli byś musiał uciec się do zwiększenia czytelności jej nazwy przez zastosowanie “underscore_naming” (nie wiem czy tak się nazywa, teraz to określenie wymyśliłem, ale chyba wiadomo o co chodzi).

Zmienne, parametry, pola, właściwości – wszystko powinno być nazwane tak, aby nie było wątpliwości do czego służy. To nie jest takie trudne.

Po drugie: testy jednostkowe. Zamiast pisać komentarz “ten kod się wywali jeśli podstawisz 3 zamiast 1 pod X bo…” – napisz testy. Jeden, który pod X podstawi 1 i wykona się jak trzeba i drugi, który pod X podstawi 3 i będzie oczekiwał wyjątku. Ten drugi test nazwij tak, aby nie było wątpliwości skąd takie a nie inne zachowanie systemu – z wyjaśnieniem w nazwie testu. Ale o tym pisałem już kiedyś w poście “Jak nazywam testy“.

Komentarze uzasadnione

Ja właściwie nigdy nie komentuję kodu i nie zdarzyło się jeszcze, aby mi komentarzy brakowało, nawet jeśli wracam do projektu po kilku miesiącach. Są jednak wyjątki…

Jeśli jesteś szczęśliwym autorem publicznie używanej biblioteki bądź frameworku to najprawdopodobniej lepiej będzie mieć nawet ścierwiaste ghostdocowe komentarze na publicznym API niż nie mieć ich w ogóle. Ludzie lubią jak im się coś pojawia w dymku Intellisense, nawet jeśli nie wnosi nic nowego. A jeżeli dorzucisz tam faktycznie przydatne informacje, jak przykłady użycia w tagu <example> albo szczególne przypadki w tagu <remarks> to tym lepiej dla ciebie. To powinno znaleźć się w dokumentacji, ale w tym przypadku komentarze (mówię oczywiście o xmlowych komentarzach) pełnią właśnie taką rolę. I lepiej żeby nie były rozjechane z faktycznym kodem.

Te XMLowe komentarze nawet w wewnętrznych projektach mają jedne zastosowanie: oferują tag <exception>, gdzie można zadeklarować wyjątki mające prawo wyskoczyć z metody. Co prawda i tutaj najprawdopodobniej wpadniemy w pułapkę out-of-sync (bo ktoś doda try/catch w metodzie ale nie zaktualizuje komentarza, albo okaże się że jednak czasami lecą także inne wyjątki). ale mimo wszystko warto o tym pamiętać i czasami stosować.

Ostatni przypadek uzasadnionych komentarzy jaki przychodzi mi do głowy do tzw. “dirty workarounds”, czyli walczenie z wiatrakami wykorzystywanych bibliotek i obchodzenie ich dziwnego zachowania. Nazewnictwo składowych programu nic tutaj nie pomoże. Podobnie jak osobna dokumentacja. Wyjaśnienie takich scenariuszy musi znajdować się w kodzie, tuż obok pokręconych struktur pomagających osiągnąć pożądany efekt.

Mam dwa przykłady:

1. Pobierałem dane z FIM za pomocą ich web service wysyłając zapytanie xpath z funkcją “contains”. Okazało się jednak, że geniusze w Microsofcie, jak to u nich czasami bywa, napisali własną interpretację tej funkcji xpath i “contains” oznaczało u nich coś innego niż powinno (w FIMie to “zwróć true jeśli których z wyrazów w przeszukiwanym stringu zaczyna się na podany string” czy jakoś tak). Oszukałem system – podczas wysyłania zapytania zamieniałem “contains(a1, a2)” na “starts-with (a1, %a2)” i działało jak trzeba, bo % jest traktowany jako normalny SQL-owy wildcard. Ale taki kod wymagał porządnego wyjaśnienia, łącznie z linkami, dlaczego czynię takie herezje.
2. Raz jeden jedyny w życiu natknąłem się na sytuację, której nijak nie potrafiłem logicznie wytłumaczyć. Dotyczyło to NHibernate i MySql – w pewnych bliżej nieokreślonych okolicznościach NH zwracał mi obiekt z bazy, ale wywalał się przy pobieraniu jego relacji mimo tego, że one tam były. Zdarzało się to raz na kilka tygodni (w kawałku kodu wykonywanym co kilkanaście sekund) i w systemie działa się przez to sodoma i gomora (użytkownicy byli bombardowani SMSami co, jak się domyślacie, nie było ani miłe ani tanie). Spędziłem nad tym masę, masę czasu, ale do niczego nie doszedłem. Skończyło się na wyłapaniu jednego konkretnego wyjątku, zbadaniu czy na 100% wystąpiła ta konkretna sytuacja i… zabiciu aplikacji przez wywołanie HttpRuntime.UnloadAppDomain(). Bardzo, bardzo brzydkie rozwiązanie, ale okazało się, że jest do zaakceptowania. I oczywiście wymagało naprawdę solidnego komentarza gdyby ktoś się kiedyś zastanawiał dlaczego rzucam na system bombę atomową.

Czekam na… komentarze;)

I to tyle co mam do powiedzenia. O ile komentarze w kodzie są be, fu i w ogóle, co starałem się udowodnić, to na blogu im więcej tym lepiej oczywiście. Więc jak to wygląda u Was?