Style Guide i Coding Standard – czy to już jakość?

ucgosu.pl 4 lat temu

Kiedy zdamy już sobie sprawę, iż sprytny kod i mikrooptymalizacje to nie jest dobra droga, dochodzimy do wniosku, iż jakość kodu jednak ma znaczenie. A wyrazem dbania o tą jakość kodu jest wprowadzenie odpowiednich reguł. Temu właśnie służą Style Guide i Coding Standard. W C są one szczególnie ważne, bo język pozwala nam na wiele dziwnych rzeczy. Czasem występują jako dwa oddzielne dokumenty, a czasem jako jeden. Różni się również sposób ich użycia. Czasem zawierają rozsądny zestaw zasad, który faktycznie jest przestrzegany w projekcie. Czasem natomiast są tak restrykcyjne i tak niemożliwe do wyegzekwowania, iż nikt choćby nie próbuje. Kiedy indziej natomiast każdy i tak wie lepiej. W dwóch ostatnich przypadkach reguły służą głównie do chwalenia się na zewnątrz, iż do naszego projektu DODALIŚMY jakość. No właśnie to tak nie działa. Możemy wprowadzić te wszystkie reguły a i tak pisać w sposób zupełnie niezrozumiały dla innych.

Dlatego dzisiejszym artykule porozmawiamy sobie właśnie o zasadach znajdujących się zwykle w tego typu dokumentach. Po co powstały i jak je egzekwować. Oczywiście temat jest tak rozległy, iż omówię tylko mały odsetek najczęściej spotykanych reguł.

Style Guide

Style Guide to zestaw konwencji przyjętych dla danego projektu obejmujący takie tematy jak nawiasy, spacje, ilość linii czy sposób nazewnictwa. Już w latach osiemdziesiątych badania wyraźnie pokazały, iż o ile trzymamy się tych samych konwencji w całym projekcie, zmniejszamy ryzyko popełnienia błędu. Po szczegóły odsyłam do książki „Code Complete”.

To chyba nie powinno nikogo dziwić. o ile trzymamy się jednej konwencji nawiasów, funkcje i zmienne nazywamy według jakiegoś schematu, a pliki i foldery są ułożone w jakiś logiczny sposób, to po prostu łatwiej poruszać się nam po kodzie i wiemy gdzie się czego spodziewać. I tutaj naprawdę nie ma znaczenia wyższość jednego stylu nad innym. Ważne tylko, żeby być systematycznym i się tego trzymać. Poza tym o ile zespół nie potrafi się dogadać w tak elementarnej kwestii, to już jest naprawdę słabo.

Na końcu artykułu podam kilka linków do przykładowych Style Guide’ów. Natomiast najpierw zastanówmy się nad genezą niektórych popularnych punktów.

Limit 80 znaków w linii.

To ograniczenie było podyktowane ograniczeniami hardware-owymi w latach 70-tych. Monitory i drukarki z tamtych czasów nie potrafiły zmieścić więcej znaków w jednej linii. Ciekawe, iż to ograniczenie zachowało się do dzisiaj. Okazuje się jednak, iż w dzisiejszych czasach również warto ograniczać ilość znaków w linii tak aby np. diffy dwóch czy choćby trzech wersji mieściły się na ekranie. Poza tym pojedynczą linię kodu powinniśmy być w stanie w całości objąć wzrokiem. Znając aktualne założenia możemy lekko zmodyfikować ten limit 80 znaków, ale jest on zadziwiająco bliski optimum.

Limit linii w funkcji

To ograniczenie z kolei było podyktowane ilością linii w starych terminalach (24 linie). Chodziło o to, aby funkcja mieściła się na jednym ekranie bez przewijania. Dzisiaj tego ograniczenia już nie mamy, ale nie znaczy to oczywiście, iż funkcje mogą być nieskończenie długie. o ile stosujemy Single Responsibility Principle i funkcja zawiera tylko operacje na jednym poziomie abstrakcji to i tak nie powinna przekraczać kilkunastu linii.

Limit linii w pliku

Zwykle limity wynoszą około 2000 linii. To ograniczenie ma sprawić, iż pliki nie są takimi worami, gdzie możemy wrzucić wszystko. Po raz kolejny powinniśmy trzymać się reguły SRP i wprowadzić odpowiedni podział plików ze względu na funkcjonalność. Jednak podział ten powinien wynikać z projektu systemu, a nie z faktu, iż zbliżamy się do 2000 linii. Bo to absurdalne, o ile plik na 1900 linii jest zupełnie w porządku, a 2100 już oznacza zaniedbanie jakości. Szczególnie o ile zmiana rozmiaru wynika np. z dodania Doxygenów.

Oczywiście każdy przypadek trzeba rozpatrzyć indywidualnie. Zbliżanie się do limitu linii jest na pewno podpowiedzią, iż coś może być nie tak i powinniśmy ponownie zastanowić się nad projektem tego modułu. Być może da się to sensownie podzielić, a może pozostawienie trochę większego pliku jest w porządku. Jednak na pewno nie powinniśmy robić jakiś bezsensownych zmian tylko po to, żeby na siłę spełnić ograniczenie.

Reguły dotyczące białych znaków

Tak naprawdę z punktu widzenia kompilatora białych znaków mogłoby nie być w ogóle (dlatego na przykład w obfuscated code contest często ich nie ma). One są dla nas – dla ludzi czytających kod. Reguły więc skupiają się na zapewnieniu czytelności i uniemożliwieniu robienia prawdziwych głupot. Wiecie na przykład, iż w C można zapisać operator odwrotnej strzałki?

a<-b;

Jednak o ile zapiszemy to zgodnie z zasadami dotyczących spacji, od razu nam się rozjaśni:

a < -b;

Przy okazji białych znaków pozostaje jeszcze kwestia typowej wojny taby vs spacje.

Jednak tak jak wspomniałem wcześniej – ważniejsze od konkretnego stylu jaki wybierzemy jest, żeby po prostu był wszędzie taki sam.

Nazwy zmiennych i funkcji

Tutaj też toczą się święte wojny under_score vs CamelCase. Są również dziwne pomysły typu notacja węgierska, gdzie zmienne są poprzedzone prefixami oznaczającymi typ. W dzisiejszych czasach IDE podpowiada nam typ, więc nie musimy dodawać prefixów utrudniających zapamiętanie nazwy. Ale np. WinAPI, czy FreeRTOS jej używają. Prawdopodobnie ze względów historycznych.

Przy nazwach funkcji bardzo popularne są prefixy oznaczające moduł. W C++ na przykład mamy namespace i nie musimy sobie tym zaprzątać głowy. Jednak w C jest tylko jedna przestrzeń nazw i dzięki takim prefixom od razu wiemy z jakiego modułu pochodzi dana funkcja.

O ile w przypadku funkcji API używanych na zewnątrz modułu takie nazwy z pewnością mają sens, to używanie ich w funkcjach lokalnych jest już wątpliwe. Funkcja statyczna i tak nie może być użyta w innym pliku, więc z definicji należy do aktualnego modułu.

Przy nazwach zmiennych i funkcji najważniejszy jednak nie jest prefix, czy konwencja. Najważniejsze jest, żeby nazwa jasno informowała o celu realizowanym przez daną funkcję, czy zmienną. Najlepiej łącznie z efektami ubocznymi. o ile trudno nam nadać nazwę, która oddaje całe zadanie realizowane przez funkcję, prawdopodobnie robi ona zbyt wiele.

Komentarze

Co do komentarzy istnieje popularny mit wyrażany często stwierdzeniem:

Ooo to jest dobrej jakości kod – ma dużo komentarzy.

No właśnie nie. Komentarze zwykle oznaczają, iż kod sam w sobie jest niezrozumiały i wymaga dodatkowego wytłumaczenia. Dlatego na przykład Uncle Bob w książce „Clean code” stawia zupełnie przeciwną tezę:

Każdą linię komentarza, który muszę dodać, aby kod stał się zrozumiały, traktuję jako osobistą porażkę.

Oczywiście nie chodzi mu, żeby nie pisać komentarzy do niezrozumiałego kodu i niech się potem domyślają. Uncle Bob twierdzi raczej, iż dzięki narzędzi języka takich jak typy nazwy funkcji i zmiennych itp. można wyrazić swoje intencje na tyle klarownie, iż komentarz nie wniesie żadnej nowej wartości.

Popatrzmy na przykład na funkcję:

void light_set_state(uint8 state);

Wiemy, iż steruje ona oświetleniem. Mamy jakiś argument state, który jest przez nią ustawiany. Możemy do funkcji przekazać cały zakres uint8 0-255 i nie mamy pojęcia jaka wartość odpowiada jakiemu stanowi. Domyślamy się, iż możliwe stany to ON/OFF. Ale równie dobrze może to być płynna regulacja 0-100%. A może pełen zakres 0-255? Bez komentarza nie jesteśmy w stanie tego stwierdzić.

Drugi przypadek mówi nam już o wiele więcej:

enum light state { LIGHT_STATE_OFF, LIGHT_STATE_ON, }; void light_set_state(enum light_state state);

Tym razem argumentem jest enum, który ma tylko dwie możliwe wartości. Dużo trudniej popełnić błąd, prawda? I żaden komentarz nie jest nam potrzebny.

Coding Standard

Coding Standard to dokument opisujący zalecane i zakazane konstrukcje języka. W C jest szczególnie ważny, bo możemy w ten sposób zakazać robienie wielu dziwnych rzeczy. Czasem Coding Standard i Style Guide tworzą jeden dokument. Jest to praktyczne, ponieważ w niektórych elementach styl może pomagać w eliminowaniu złych praktyk. Jak na przykład wcześniejszy przykład z brakiem spacji i odwróconą strzałką.

Istnieją proste standardy opisujące podstawowe rzeczy i bardzo szczegółowe jak na przykład MISRA C. Jakie punkty często tam się znajdują?

Nawiasy klamrowe przy pętlach i ifach

Na przykład aby przy ifach i pętlach zawsze stosować klamry, a nie jednolinijkowce. W ten sposób możemy uchronić się przed takimi problemami:

if(TRUE == stop) flag = ON; interlock = ON; if(ON == interlock) open_doors(); else apply_breaks(); sound_alarm();

Wcięcia sugerują wykonanie dwóch operacji w pierwszym ifie i dwóch w ostatnim else. Jednak przez brak nawiasów klamrowych drugie operacje w obu przypadkach wykonają się niezależnie czy warunek jest spełniony, czy nie. Tak więc w tym przypadku interlock = ON wykona się niezależnie od zmiennej stop, drzwi się zawsze otworzą i zawsze odezwie się alarm. Chyba nie o to chodziło autorowi?

Zapewne chodziło o coś takiego:

if(TRUE == stop) { flag = ON; interlock = ON; } if(ON == interlock) { open_doors(); } else { apply_breaks(); sound_alarm(); }

Dlatego zawsze stosując klamry, choćby do jednolinijkowców, możemy łatwo uniknąć takich błędów. Szczególnie, iż często w późniejszym czasie kod może być rozbudowywany.

Zmienna jako prawy argument operacji porównania

Bardzo częstym błędem jest literówka myląca operację przypisania (=) i porównania (==). Może to mieć poważne konsekwencje:

Zmienna się przypisuje zamiast porównać i warunek zawsze przyjmie tą samą wartość. Aby się przed tym obronić wyznaczy zamienić strony porównania:

if (true = isCrazyMurderingRobot)

Wtedy stała jest po lewej, a zmienna po prawej. I jak zrobimy literówkę, zakończy się ona błędem kompilacji.

Więcej tego typu zasad można znaleźć w linkach na końcu artykułu. Pisałem też o tym kiedyś, dawno temu w artykule na Forbocie.

Jak egzekwować te zasady

Narzędzia

Ogólna zasada jest prosta – jak najwięcej z tych zasad staramy się egzekwować automatycznie. Mogą nam w tym pomóc narzędzia do statycznej analizy kodu i do formatowania takie jak:

Pomocne może być także włączenie odpowiednich flag kompilatora. Niektóre kompilatory mają choćby warningi z MISRA C.

Jeżeli używamy narzędzi bardzo ważne jest kilka zasad:

Mamy oficjalne statystyki i warningi z CI.
Konfiguracja narzędzi jest w systemie kontroli wersji razem z kodem.
Możemy odtworzyć identyczną analizę na maszynie developerskiej.
Zmiany nie wchodzą do głównego brancha przed wyjaśnieniem warningów.

Oczywiście możemy sobie dodawać jakieś pre-commit hooki i inne cuda, żeby dodatkowo ulepszyć nasz system. Ale najważniejsza sprawa to mieć łatwy dostęp do oficjalnych wyników analizy i potrafić odtworzyć identyczną analizę lokalnie.

Code review

Automatyczne narzędzia mają to do siebie, iż czasem da się je oszukać. Niektórych błędów mogą nie zauważyć, inne natomiast zgłaszać jako false-positive. Dlatego dodatkowym krokiem powinno być code review.

Główna część code review powinna się skupiać na designie, czytelności i tego typu rzeczach niemożliwych do sprawdzenia przez automaty. Ale zawsze przy okazji recenzent może zauważyć także błędy, których nie dało się sprawdzić automatycznie albo potwierdzić poprawność uzasadnienia, iż znaleziony błąd to false-positive.

Ale tutaj ważna jest jedna kwestia – review nie może być zdominowane przez dyskusje o spacjach i nawiasach! 90% pracy powinny wykonać automaty. To bez sensu o ile dyskutujemy o spacjach, nawiasach, prefixach w nazwach i doxygenach, a umykają nam poważne błędy! Często tak właśnie wyglądają review. I koniecznie musi wszystkie robić najbardziej doświadczony senior w zespole. Bo tylko on jest wystarczająco kompetentny do decyzji o spacjach i prefixach. Brzmi jak absurd, prawda?

Jakości nie można „dodać na końcu”

Częste podejście do Coding Standardu i analizy statycznej to odkładanie wszystkiego na koniec. Najpierw zrobimy całą implementację, a potem będziemy „dokręcać”… jak starczy czasu oczywiście.

No właśnie nie. Takie podejście nie ma sensu. Potem dostaniesz jakiś błąd z analizy i nie będzie Ci się chciało wszystkiego zmieniać. Dodasz tylko jakieś rzutowanie, czy zmienną pomocniczą żeby ominąć warning. Albo napiszesz „suppressa”, czy „justify” z uzasadnieniem dlaczego to false-positive.

A potem stwierdzisz, iż statyczna analiza i te wszystkie reguły są bez sensu, bo tylko utrudniają pisanie a do niczego się nigdy nie przydają.

A to wszystko dlatego, iż nie da się na koniec nadrobić wszystkich zaniedbań zbierających się od początku projektu. Musisz to potraktować jako proces i analizować wskazania analizy statycznej na bieżąco. Poza tym musisz być bezwzględny w review. Żeby każdy wiedział, iż nie może pójść na skróty i nic się nie prześlizgnie. o ile nie wyrobicie w zespole kultury dbania o jakość, to choćby ludziom, którym zależy, w końcu się odechce.

Jeżeli nie potraktujemy tego jako proces, tylko wybiórczo będziemy sobie przypominać o „dodawaniu jakości”, zrobimy z tego farsę. Realna jakość w żaden sposób się nie podnosi, a developerzy mają ileś dodatkowych zasad, które nic nie dają i nie mają żadnego sensu. Tacy developerzy pewnie dwa razy pomyślą zanim następnym razem upomną się o zwiększenie jakości. Bo już będą wiedzieć, iż to jak podpisanie cyrografu z diabłem, gdzie niby wszystko jest, ale w krzywym zwierciadle. I mogą dostać taką jakość, iż im się odechce.

Jakości nie można dodać, tylko trzeba o nią dbać przez cały czas. Nie możemy traktować tego jako task w Jirze, który można sobie zacząć w późniejszym czasie, akiedy jest skończony, nie trzeba już do tego wracać. To tylko korporacyjny sposób na pokazanie, iż coś robimy, jednocześnie nie rozwiązując problemu.

A samo istnienie Style Guide i Coding Standard opisujące zasady pisania kodu w projekcie to nie jest jakiś wyśrubowany poziom jakości, tylko minimum przyzwoitości. Chociaż oczywiście wszystko zależy od technik opisanych w tych dokumentach i na ile pokrywają się z rzeczywistością.

o ile chcesz dowiedzieć się więcej o tym, jak pisać dobry kod w C – zapisz się na newsletter przez formularz poniżej. Przygotowuję właśnie szkolenie online “C dla zaawansowanych” i na liście mailowej informacje na ten temat na pewno Cię nie ominą.