Dużo się na ten temat pisze i dużo jest też pytań. Dla mnie kwestia jest dość prosta. Weźmy na przykład ten fragment kodu, który kiedyś popełniłem – wygląda on na produkcji dosłownie tak samo:

var septets = new List<int>();
int currentSeptet = 0;
int shiftBits = 0;

for (var i = 0; i < bytes.Length; i++)
{
    currentSeptet |= bytes[i] << shiftBits;
    shiftBits += 8;

    while (shiftBits > 7)
    {
        septets.Add(currentSeptet >> _shift);

        currentSeptet >>= 7;
        shiftBits -= 7;
    }
}

Tak naprawdę patrząc na niego ciężko jest wywnioskować co on robi – nazwa funkcji, której nie podałem już powie trochę więcej. Ale jest to jakiś konkretny algorytm, który wykonuje konkretną pracę. Teraz jakie jest moje spojrzenie na dokumentowanie kodu:

  • Mógłbym dokładnie opisać każdą linijkę co robi i dlaczego za pomocą inline comments, dzięki czemu po roku jak ktoś wyśle mi buga będę wstanie dość szybko przypomnieć sobie jak to i dlaczego tak to działa.
  • Mógłbym również w dokumentacji funkcji opisać dokładnie działanie algorytmu a następnie jego implementację pozostawić bez dokumentacji. Dzięki czemu tak jak poprzednio będę wstanie skumać szybko co jest nie tak jeżeli błąd wystąpi.
  • Mógłbym również pozostawić kod bez dokumentacji i zdać się na nazwę metody jako opis co powinien robić algorytm – na przykład DecodeText. W tym jednak jest jeden problem, jeżeli algorytm jest specyficzny dla rozwiązania to będzie ciężko potem coś z nim zrobić, jeżeli zaś algorytm jest otwarty i dostępny na sieci, to nazwa DecodeText za dużo nie powie, prędzej Decode14BitText już może powiedzieć coś więcej.
  • Mógłbym opisać tylko najbardziej kluczowe miejsca w algorytmie – na przykład dlaczego sprawdzam wartość shift? A tym bardziej opisać takie miejsca, które nie są zgodne z oryginalnym algorytmem gdyż API firmy XYZ ma specyficzne przestawianie danych w porównaniu do innych firm, przez co cały algorytm musi ulec zmianie.

Dwa pierwsze podejścia nie tylko mi ułatwiają pracę jak i także osobie, która będzie musiała usiąść do tego kodu kiedy ja nie będę mógł, lub ja już nie będę pracował dla danego klienta – jednak nie są one przezroczyste, kod będzie nie ładnie wyglądał oraz jego modyfikacja spowoduje kolejne komplikacje jeżeli chodzi o komentarze (czy usunąć ten komentarz, czy go zaktualizować, albo zostawię to sobie na później). Oczywiście pojawia się pytanie, czy my chcemy dać możliwość pisania kodu komukolwiek czy wolimy by się do nas firma zwróciła w celu poprawny kodu?

Podejście trzecie zaś jest domyślnym zachowaniem, zamiast nazywać funkcję AAAA nazywamy ją konkretnie – co jest nawet chyba best practices jeżeli chodzi o nazewnictwo funkcji. Jednakże nazwa nie gwarantuje ani nam ani komukolwiek innemu możliwości „szybkiego” zapoznania się z kodem i go zrozumienia.

Jeżeli chcemy by kod mógłby być dalej rozwijany to w kluczowych miejscach gdzie wykonywana jest jedna z podstawowych operacji dla całego systemu i jest ona na tyle skomplikowana, że kod sam siebie nie wytłumaczy to ja osobiście piszę komentarze. Jeżeli zaś nie chcemy by ktoś inny się tym zajmował to możemy po prostu komentarze olać.

Ale oczywiście to też ma potem na mnie ujemny wpływ – kiedy naprawdę przyjdzie problem z moim kodem, który był skomplikowany obliczeniowo i teraz nie działa, ja będę musiał poświęcić 2 razy więcej czasu na jego sobie przypomnienie.

Dlatego ja zawsze podchodzę do tego umiarkowanie, raz napiszę dokumentację, raz jej nie napiszę, ale zawsze w miejscach kluczowych napiszę jedną linijkę z opisem dlaczego, by w razie co móc to potem poprawić – to jest właśnie rozwiązanie nr cztery.

Wracając do przykładowego kodu można by to zrobić na przykład tak:

var septets = new List<int>();
int currentSeptet = 0;
int shiftBits = 0;

for (var i = 0; i < bytes.Length; i++)
{
    currentSeptet |= bytes[i] << shiftBits;
    shiftBits += 8;

    while (shiftBits > 7)
    {
        // HACK: When we are using shift (1) we have one dump byte (it could be CR)
        septets.Add(currentSeptet >> _shift);

        currentSeptet >>= 7;
        shiftBits -= 7;
    }
}

Przynajmniej teraz jak siądę do kodu, to będę wiedział, po co mi to przesunięcie jest potrzebne. Reszta zaś jest spełniona przez punkt 3 – odpowiednia nazwa metody i lub <remarks> z linkiem do algorytmu opisanego słowami. Jest to rozwiązanie, które nie tylko powoduje, że kod wciąż jest przejrzysty ale także zachowujemy się fair w stosunku do klienta.

A wy jak podchodzicie do komentowania kodu?

PODOBNE ARTYKUŁYWIĘCEJ OD AUTORA

15 KOMENTARZE

  2. Uzywam wariant TRZECI – w moim kodzie nie ma miejsca na komentarze. Oczywiscie robie wyjatki gdy pisze API jakiegos frameworka. W tym wypadku opisuje metody publiczne, interfacy i klasy. Ale jak czesto to sie zdarza?

    Z doswiadczenia wiem ze komentarz taki jaki ty wpisales pozostanie nawet gdy kod sie zmieni.

    Implementacja metody i tak nie jest moim zdaniem przejzysta. Zauwaz ze akurat podany komentarz mi nie pomogl w zrozumieniu metody.

    Zamiast komentarza lepiej urzyc metody o nazwie considerShiftByOneForCR().

  4. tak Misiek nie wiedziales bo nie podalem nazwy funkcji z pewnych powodow. gdybys mial funkcje i jej <remarks> to bys mial linka do algorytmu. Zas metoda considerShiftByOneForCR() to by bylo juz kompletnie zbedna operacja w szczegolnosci ze jej implementacja to return param >> _shift; gdyby miala byc tam logika to co innego. To to samo prawie co checkIfNull(obj); takie samo zachowanie.

  6. Nazwa funkcji to jest podstawa i przeważnie na tym się kończy. Chociaż raz zapisałem sobie w komentarzu algorytm z podpunktami, a potem każdą linijkę opatrzyłem numerkiem z tego głównego komentarza. Łatwiej mi było tak ten algorytm ogarnąć – zwłaszcza, że wcześniej robiłem błąd, pisząc kod z głowy. A że VS pozwala zwijać komentarze, to wcale się nie przejmowałem tymi 10cioma linijkami komentarza w środku funkcji ;-)

    A opisywanie każdej linijki jest zbędne, bo sama linijka jest tylko linijką – liczy się całość. Chyba, że gdzieś się wykorzystało jakiś ‘trick’ – wtedy miło jest to opisać ;-)

  7. No wlasnie nie zbedna. Przeciez przyznasz ze linijka

    considerShiftByOneForCR()

    jest duzo bardziej czytelna niz

    septets.Add(currentSeptet >> _shift);

    Technika bardzo uzyteczna aby uzyskac kod czytelny i zrozumialy bez zadnych komentarzy. Idealna sprawa. Kod sie pozniej czyta jak ksiazke.

  8. szczerze nie Misiek, linijka consiferShiftByOneForCR wcale nie pomaga, bo moze to byc CR albo i nie. mozna napisac considerShiftByOneForDumpChar ale znow to jest tylko i wylacznie przeniesienie tego do funkcji – 4 linijki zamiast prostego opisu. nie widze sensu w tworzeniu funkcji tylko po to by zwrocic wartosc operacji.

    z jedna linijka komentarza latwiej mi sie kod czyta niz w przeskakiwaniu z funkcji do funkcji by sie skapnac jak on dziala. Pewnie przy projekcie code bubbles to by mialo rece i nogi (http://www.cs.brown.edu/people/acb/codebubbles_site.htm ) ale tak to bardziej komplikuje niz ultawia.

  9. Widze ze sie nie przebije, ale zachecam do przemyslenia pod czas lektury np.

    http://memeagora.blogspot.com/2008/11/comments-code-smell.html

    Proponuje lekture Kent Beck (chyba nie trzeba przedstawiac, choc tutaj posrod .Netowcow nigdy nie wiadomo ;) – Implementation Patterns a wszczegolnosci Composed Method Pattern.

    Mozna tez pogooglowac Composed Method Pattern lub Comments are Code Smells. Naszczescie w Javie te podejscie sie juz stalo czescia najciekawszych podejsc programowania jak XP, Agile, SCRUM i co tam by jeszcze nie bylo.

    Aby tez niebawem w .NET. Trzymam kciuki chlopaki ;)

  10. Misiek, moim zdaniem wszystkie wzorce sa dobre jezeli znajduja swoje zastosowanie. Ale takze jestem przeciwnym naduzywaniem wzorcow tylko po to by je uzyc. I tak jak CMP ma sens w niektorych przypadkach tak w niektorych nie ma. tak samo jak ze wszystkimi innymi wzorcami – nalezy je uzywac madrze i rozsadnie, a nie ladowac je wszedzie gdzie popadnie.

    Nie wiem tez czemu wiazesz CMP z metodykami zarzadzania projektami – dla mnie jedno nie rowna sie drugiemu, ale to moje zdanie.

    PS.: bloga Neala dobrze znam :)
    PS2.: a co do przebicia sie – nigdy nic nie wiadomo Misiek :)

  11. zeby nie bylo "metodykami zarzadznia projektami" mialem na mysli "zarzadznie procesem wytwarzania oprogramowania", zastosowalem niefortunny skrot myslowy

  12. No to wkoncu doszlismy do wspolnego zdania. Aby zacytowac Kenta:

    There is no universal rule. Programmers need to think, communicate, and learn. That’s part of being professional.

  13. Jak dla mnie nazwa metody oraz jej dokumentacja to priorytet, ponieważ niezależnie czy tworzę dll z logiką, czy też kod w głównej aplikacji to te elementy widać "wszędzie".
    Co do komentowania kodu wewnątrz metody to uważam iż rozsądnie jest uskuteczniać komentarze w kluczowych lub też mało zrozumiałych liniach, czyli raczej pkt 4.

  14. Ja na przykład komentuję wszystko na potęgę. Za dużo razy zdarzało mi się przejąć kod kompletnie nie opisany. Czasem jest to komentowanie na prawdę oczywistych kwestii, ale nigdy nie wiadomo, kto odziedziczy kod po tobie…

  15. tez tak kiedys robilem – kazda metoda miala ladna dokumentacje itp. potem przestalem i robie to jedynie dla bibliotek lub frameworkow, dla logiki biznesowej i konkretnej aplikacji pozostaje przy dobrej nazwie metody i wrazie co komentarzach do krokow w kluczowych miejscach.

    a przestalem bo w ktoryms momencie mialem 100 linii kodu i 200 komentarzy :) stwierdzilem ze zaczalem przesadzac :)

Comments are closed.