Krok 2 | Komentarze po całości – 5 kroków do zagłady
Poprzednio dowiedziałeś się co się stanie jak postanowisz ładnie zamknąć całą funkcjonalność w jednej funkcji. Próbowałem Cię przekonać, że to nienajlepszy pomysł. Ale przecież Ty jesteś tym doświadczonym programistą! Kod ma być zwięzły i niepozostawiający wątpliwości. A jak kod ma być zwięzły ale jednocześnie czytelny to jest tylko jedno rozwiązanie – komentarze. Nie zostawiajmy przyszłych kolegów samym sobie i ułatwmy im pracę, mam rację?
Wpis jest częścią cyklu „5 kroków do zagłady”. W tym momencie dostępne wpisy to:
Seria dostępna jest też w formie wideo na moim kanale w serwisie Youtube.
Więc nie róbmy sobie żartów. Poważny kod, którego nie będzie wstyd pokazać potem kolegom powinien być dobrze UDOKUMENTOWANY. Na szczęście ludzie tworzący języki programowania wiedzieli, że to nie jest zabawa tylko poważne narzędzie i dodali komentarze. Dzięki temu nie trzeba pisać osobnej dokumentacji. Skorzystajmy z tego dobrodziejstwa i w funkcji z poprzedniej części zostawmy w kodzie informacje dla potomnych:
/// <summary>
/// Saves user in database. Before it checks if the parameters aren't empty.
/// Checks also if the login doesn't exist in database.
/// After that change password to hash value.
/// When user is saved function sends an email to user so he can confirm email address.
/// </summary>
/// <returns>
/// Id of new created user.
/// </returns>
/// <exception cref="System.ArgumentException">
/// Thrown when any of input value is null or empty or when login exists in database.
/// </exception>
/// <param name="firstName">User's first name. Can't be empty.</param>
/// <param name="lastName">User's last name. Can't be empty.</param>
/// <param name="email">User's email address. Can't be empty.</param>
/// <param name="login">User's unique login. Can't be empty.</param>
/// <param name="password">User's password. Can't be empty.</param>
public long SaveUser(string firstName, string lastName, string email, string login, string password)
{
// Check if first name is empty.
// If true throw exception.
if (string.IsNullOrEmpty(firstName))
throw new ArgumentException();
// Check if last name is empty.
// If true throw exception.
if (string.IsNullOrEmpty(lastName))
throw new ArgumentException();
// Check if email address is empty.
// If true throw exception.
if (string.IsNullOrEmpty(email))
throw new ArgumentException();
// Check if login is empty.
// If true throw exception.
if( string.IsNullOrEmpty(login))
throw new ArgumentException();
// Check if password is empty.
// If true throw exception.
if (string.IsNullOrEmpty(password))
throw new ArgumentException();
// Create database context with default connection string
using (var context = new DbContext())
{
// Check if user with passed login doesn't exist in database.
// If exists throw exception.
if (context.Users.Any(x => x.Login == login))
throw ArgumentException();
// Create hash from password.
// Use default hash algorithm.
var hashedPassword = Hash.Create(password);
// Create entity object which will be saved in database.
// Entity contains also Id field which will be filled after save changes.
var newUser = new User()
{
Login = login,
Email = email,
Password = hashedPassword,
FirstName = firstName,
LastName = lastName
};
// Add new user to list of users in db context
context.Users.Add(newUser);
// Save changes.
// After that field Id in user entity object will contain id of new row in Users table in database.
context.SaveChanges();
// Send an email to new user.
// Use passed email address.
_mailService.Send(email,
// Email title is "Potwierdź email"
"Potwierdź email",
// Create message body.
// Message contains link which allows confirm email address by user.
// Url contains one parameter where user's id is placed.
string.Format("Aby potwierdzić swój adres email kliknij w ten link: https://korpo-soluszyn.com/users/confirm/{0}", user.Id));
// Return new user id.
return user.Id;
}
}
I teraz to jakoś wygląda! Nikt! Powtarzam – NIKT! – nie zarzuci nam teraz, że nasz kod nie jest udokumentowany albo jest nieczytelny.
Nawet osoba nieumiejąca programować odnalazłaby się w tym kodzie. Teraz korzystając z takich funkcji reszta zespołu doskonale wie czego używa. Żadnego domyślania się! Wszystko dokładnie opisane. Cokolwiek się będzie zmieniać to wystarczy nie być głupim i czytać to co jest napisane nad zmienianym kawałkiem i też to zmienić. W końcu to nic trudnego dodać albo usunąć parę wyrazów.
Problem
Wygląda znajomo? Spotkałeś się z takimi epopejami w Waszym kodzie? Mi się zdarzyło. Ktoś zdecydowanie przesadził i doszło do poziomu absurdu kiedy pisanie dziwnego, niepotrzebnie zakręconego kodu spokojnie przechodziło code review. Ale jak w tym kodzie brakło komentarza do jakiejś linii albo nie każda metoda miała pełną dokumentację w komentarzach (nawet prywatna) to podnosił się krzyk i płacz na cały zespół.
Mimo, że w tym wypadku nie wpływamy na działanie czy strukturę kodu to możemy w tym kodzie dużo namieszać. W jaki sposób? Dowiesz się tego właśnie teraz.
Komentarze nie grają
Bycie nastawionym na komentowanie każdej linijki kodu powoduje, że zaczynamy być leniwi w kwestii dbania o nazewnictwo. W końcu i tak się to opisze za pomocą komentarza. W ten sposób dostajemy funkcje czy zmienne o nic nieznaczących nazwach okraszone masą komentarzy wyjaśniających co reprezentują albo co robią.
Patrząc na treść funkcji to nie problem. Bo mamy komentarz nad jej nazwą. Ale co się dzieje kiedy takiej funkcji użyjemy?
var x = a(34);
Potrafisz powiedzieć co ta funkcja robi? Ale ale! Bez najeżdżania kursorem i czekania aż pojawi się komentarz z opisem. Widząc tylko to co powyżej. Nie jest łatwo, prawda? I właśnie dlatego ten problem nazwałem „komentarze nie grają”. Bo kiedy tylko funkcja czy klasa wyjdzie poza swoje poletko w swoim pliku to nagle się okazuje, że nie ma wsparcia komentarza. I drugi programista nie może płynnie przeglądać kodu bo przy każdej linijce musi szukać komentarza jaki się za nią kryje.
Więc pierwszy problem to brak świadomości, że komentarzy nie użyjemy do czytania kodu, w którym używamy elementu jaki komentują. Musielibyśmy w takim wypadku napisać kolejny komentarz, który opisywałby to samo co komentarz w pliku z funkcją. I w ten sposób wpadamy w pętlę komentowania.
Upłynął termin przydatności
Komentując BARDZO DOKŁADNIE nasz kod ciągle powtarzamy, że spokojnie, będziemy dbali o to żeby komentarze były aktualne. Trwa do mniej więcej do pierwszego „muszę to szybko skończyć”.
Ten problem jest najpoważniejszy z opisanej tutaj gromadki. Bo o ile stracenie czasu na obsługę komentarzy co najwyżej wywoła w nas frustrację niewyrabiania się z zadaniami o tyle problem ich nieaktualności może nieść poważniejsze konsekwencje.
Jeden ze scenariuszy opisałem już w pierwszej części tego cyklu. Chodzi o to, że mając opisane działanie funkcji jest praktyczne pewne, że w jakimś momencie, chociażby przed zbliżającym się terminem, w końcu nie zaktualizujemy komentarza. I ktoś inny używając tej funkcji będzie zakładał, że robi ona coś innego niż w rzeczywistości.
Inny, podobny przykład, dotyczy komentowania każdej operacji, np. przypisania czy zmieniania wartości. Załóżmy, że mamy w kodzie coś takiego:
// Change status to "COMPLETED" and store in variable number of items for that status. var x = ChangeStatus(3);
Wszystko działa i ma sens. Komentarz wyjaśnia co się tutaj dzieje. I nagle ktoś robi w kodzie podmianę ChangeStatus(3) na ChangeStatus(2) . I zostajesz z takim kodem:
// Change status to "COMPLETED" and store in variable number of items for that status. var x = ChangeStatus(2);
Czy nadal opis jest poprawny? Niewiadomo. Narzędzia na refaktoringu nie zaktualizują opisu za Ciebie. Możliwe, że zmieniły się numery statusów. Albo doszła konieczność zmiany na status PREAPPROVAL zamiast COMPLETED. Dopóki nie zagłębisz się w kod to ta linijka jest niewiadomą.
Ale przecież jest komentarz więc po co drążyć temat. Napisali, że COMPLETED to znaczy, że to 2 to właśnie numer tego statusu.
Gdzie to było…
Nadmiar komentarzy zaciemnia kod. Nie ma co do tego wątpliwości. Kiedy widzę w jakimś pliku ścianę komentarzy to ciężko mi wyłapać gdzie tam w ogóle jest jakiś kod. I to w sytuacji kiedy IDE już mi koloruje składnię. Bez tego znalezienie co jest wykonywalną linijką, a co jakimś opisem graniczy z cudem. Zwłaszcza kiedy ludzie używają operatora wielolinijkowych komentarzy. Szukanie istotnych informacji w takim pliku porównałbym do czytania przepisu, w którym zamiast listy składników mamy opowiadanie na 2 strony, w którym trzeba wyłapać ile czego jest dodawane.
A podobno nie masz czasu
Twierdzisz, że ledwo się wyrabiasz z robotą ale jednocześnie masz komentarze w każdej prywatnej metodzie? Coś tu jest nie tak. I może wcale nie jest tak źle z tymi terminami? Wygląda na to, że po prostu swoje siły przyłożyłeś w miejscu, które ich nie potrzebowało.
Napisanie wielozdaniowego komentarza zajmuje czas, chociażby na myślenie co warto dodać, a co jest zbędne. Utrzymywanie takich komentarzy to jeszcze więcej wydanego czasu. W końcu trzeba po każdej zmianie dokładnie czytać to co jest w komentarzu zawarte, a potem myśleć czy nadal taki opis ma sens.
Inna sprawa to tracenie mnóstwa czasu na przebijanie się przez ścianę komentarzy wspomnianą w poprzednim punkcie. Zamiast wyłapania szybko gdzie jest interesująca nas zmienna to musimy czytać te opisy. A ponieważ, jak już udowodniłem, nie można ufać komentarzom do akcji to jak już przebijemy się przez komentarze w jednym pliku to musimy przejść do kolejnego pliku i sprawdzić czy opis, który właśnie zobaczyliśmy na pewno mówi prawdę i funkcja nie zmieniła swojego zachowania.
Przyczyna
Taki stan rzeczy wynika znowu z kilku czynników. I wybrałem trzy, które moim zdaniem są najpowszechniejsze.
Stara szkoła
Jeden z nich związany jest z nauką głównie na uczelni albo od osób, które swoją karierę zaczynały wiele wiele lat temu kiedy narzędzia nie były tak rozbudowane, a kod pisało się strukturalnie. Na zajęciach z takimi osobami, które pokazują jak np. w C++ pisać kod strukturalny, widzimy jak każą komentować każdy krok w kodzie. Łączą to zazwyczaj z jednoliterowymi albo skrótowymi nazwami. Mając funkcję a() i zmienną g faktycznie ciężko się połapać o co chodzi bez komentarza. Więc od takich osób uczymy się tego, żeby pisać nieczytelne, skrótowe nazwy i potem okraszać je toną komentarzy.
Niezrozumienie
Druga sprawa wynika z niezrozumienia przez mniej doświadczonych (niezależnie od liczy przepracowanych lat) programistów zasady, że komentarze powinny wyjaśniać o co w kodzie chodzi. Mylą oni wyjaśnianie tego dlaczego dany fragment kodu coś robi z opisywaniem tego co dany fragment kodu robi. Przez to starają się prowadzić „czytelnika” za rękę po kolejnych operacjach jakie napisali.
Chcę być jak najlepsi
Ostatnia przyczyna jest wg mnie związana z chęcią bycia „pro”. Każdy z nas widzi, że używając jakiejś funkcji czy klasy z różnych bibliotek nasze IDE wyświetla pomocne, opisujące sytuację komentarze. Wtedy też dochodzimy do wniosku, że tak musi być to zrobione żeby kod był profesjonalny. Tylko, że nie zastanawiamy się skąd to wynika, w jakich sytuacjach ma to sens i jakie niesie za sobą konsekwencje. A ma to sens w sytuacji kiedy udostępniamy kod szerokiej grupie odbiorców, głównie tych, którzy nie mogą zerknąć w kod źródłowy tego z czego korzystają.
W dodatku twórcy bibliotek komentują w taki sposób przede wszystkim to co jest publiczne. I robią to niejako z przymusu bo sami wewnętrznie nie potrzebują zazwyczaj tych komentarzy. Ułatwiają pracę tym, którzy korzystają z ich dzieła ale płacą za to cenę w postaci dużej trudności w utrzymaniu takiego kodu. W końcu jeżeli ktoś nie chce być oskarżony o niedbalstwo to musi przed każdym releasem przejrzeć wszystkie te publiczne komentarze i sprawdzić czy odzwierciedlają bieżącą sytuację. Kiedy popatrzycie na publiczne repozytoria to nieraz znajdziecie w nich commity z wiadomością „aktualizacja komentarzy w klasie X”.
Rozwiązanie
Daleki jestem od mówienia, żeby komentarzy nie pisać w ogóle. Jednak żeby miały sens i nie przysparzały masy problemów trzeba się trzymać kilku zasad.
Zadaj sobie jedno ważne pytanie
Przede wszystkim komentarz ma odpowiadać na pytanie „Dlaczego?” ale absolutnie nie może odpowiadać na pytanie „Jak?”. Już ten punkt zawęża pulę komentarzy do tych, które opisują zwykle dwie sytuacje – dlaczego użyłeś tutaj tego hacka albo dlaczego zastosowałeś w tym miejscu ten algorytm. Często wręcz taki komentarz sprowadza się do jednego zdania i linku do StackOverflow albo innego źródła, które opisało problem.
Nie opisuj, że używasz jakiejś funkcji skoro właśnie jej używasz. To bez sensu.
Name-it!
Kiedy masz ochotę opisać jakąś funkcję czy klasę komentarzem zrób sobie najpierw prostą check-listę. Dzięki niej dojdziesz do wniosku, że chciałeś wstawić komentarz bo masz inny nierozwiązany problem z kodem. Tak więc po kolei:
- Spróbuj nazwać funkcję, klasę albo zmienną tak żeby mówiła za co odpowiada. Np. „ZapiszUżytkownika”, „SumaWydatków”, „Zamówienie”.
- W pierwszym punkcie wychodzi Ci, że Twoja funkcja musi się nazywać „SprawdźCzyLoginIstniejeIDodajNowegoUzytkownikaAlboZaktualizujIstniejącegoWysyłającMailaOZmianie”? Masz problem opisany w części pierwszej tego cyklu. Rozwiąż go i wróć do poprzedniego punktu.
Wyeliminowanie problemu z nazewnictwem bardzo często eliminuje też konieczność wstawiania komentarzy. Bo nazwa sama wyjaśnia jak coś działa lub za co odpowiada.
Tak samo to działa w przypadku przekazywanych danych. Wyeliminowanie magicznych wartości, takich jak pokazany wcześniej numer status, eliminuje konieczności komentowania ich. Zamiast przekazywać surową liczbę obudują ją w stałą albo enum o odpowiedniej nazwie. Wtedy zamiast kodu, który trzeba wyjaśniać:
var x = ChangeStatus(3);
Dostaniesz kod, który sam się opisuje:
var completedItemsCount = ChangeStatus(Statuses.Completed);
Nie wchodź w szczegóły
Jesteś w tej małej grupie osób, które tworzą własne publiczne biblioteki lub frameworki? Albo w Waszym projekcie szefostwo lub przepisy wymagają dokumentowania kodu? Nie ma problemu. Ale zastanów się nad poziomem zagnieżdżenia! Bo czy dodanie pełnej dokumentacji w komentarzu dla prywatnej funkcji pomocniczej ma sens? Nie może być ona użyta nigdzie na zewnątrz, a sama w sobie stanowi tylko fragment jakiejś publicznej metody. Jest jednym z klocków, z których składa się to co wystawiasz na zewnątrz, a samo w sobie nie ma wartości większej niż możliwość uporządkowania kodu w klasie. Zostaw ją w spokoju.
Leave a Comment