Był rok 2011 - konkretnie lato lub jesień. W tle rozpoczynała się restrukturyzacja firmy a na pierwszym planie trwały batalię o termin oddania kolejnej wersji produktu. Standardowo jak to w takich sytuacjach bywa funkcjonalność na deadline była absolutnym priorytetem a kod to tam sobie posprzątamy później w wolnym czasie. Zaczęły pojawiać się nawet takie dziwne i egzotyczne pomysły w stylu "nie piszcie testów to według obliczeń dostarczycie 30% szybciej" albo " teraz szybko zaimplementujcie a jak będzie sukces to później zrobi się sprint lub dwa na refaktoring". Oczywiście gdy kończy się implementować jeden "urgent ficzer" wtedy okazuje się, że to był tylko taki przedsionek dla kolejnej niezbędnej funkcjonalności.
Cóż, każdy z nas(w zespole) przeżył pracę z klasami liczącymi 10000+ linii kodu i dobrze wiedzieliśmy do czego prowadzi patologia "poprawimy później". Dlatego praktyką zespołu było i jest ciągłe sprzątanie po sobie. Dzięki temu byliśmy w stanie dostarczyć kolejne super urgent ficzery w 2012 i 2013. Jednocześnie pojawił się postęp kulturowy kiedy to użytkownicy zwani biznesowymi zaczęli przejawiać zrozumienie w kierunku konceptu "dostarczamy w miarę regularnie bo pilnujemy wysokiej jakości kodu". To był taki krok pierwszy w kierunku wysokiej higieny kodu - zrozumienie, że jak się nie posprząta gówna to będzie śmierdzieć. Czas teraz na kolejny krok czyli dlaczego tak często trzeba latać do kibla - (kontynuując metaforę - trzeba zrozumieć co jemy i jak to wpływa na żołądek)
Tyle tytułem epicko-kulinarnego wstępu.
Jaka jest właściwie złożoność kodu?
Oczywiście każdy z nas czytał "czysty kod" nawet po kilka razy. Mimo tego co jakiś czas musimy odwiedzić wyniki Sonara aby zobaczyć czy naruszenia kodu znowu nie pikują w okolicach akceptowalnych granic. W naszym projekcie sformułowanie "o kurwa" pojawia się przy około 50 naruszeniach - w innych projektach punkt niemiłego zaskoczenia może znajdować się gdzieś indziej. I chociaż wspomniany "Czysty kod" to doskonała podstawa zawierająca wiele reguł, które każdy programista powinien wbić sobie do głowy (może wtedy zmienne w stylu "temp1" czy "zzz" zniknęłyby z kodu) to jednak aby uzyskać intuicyjne zrozumienie złożoności kodu być może musimy zejść trochę głębiej.
Tutaj dla analogii także przemycę fakt żywieniowy. Podobnie jak ogólne rady "pisz czysty kod, małe metody i czytelne nazwy" wyznaczają ogólny kierunek poszukiwań tak samo rady "jedz zdrowo i dużo się ruszaj" są tylko początkiem. Trzeba zejść głębiej aby zrozumieć jak działają poszczególne składniki. Przykładowo na półce w sklepie stoi sobie taka buteleczka soku z napisem slim za 2,99. Producent mógł tam napisać duże "slim" czy "odchudzanie" ponieważ dodał śladowe ilości L-karnityny, która w niezależnych badaniach naukowych faktycznie ujawniła właściwości odchudzające ALE... po pierwsze L-karnityna pomaga w transporcie tłuszczy w trakcie wysiłku czyli jeśli nie ruszy się dupy to i tak ten soczek w niczym nie pomoże... a co śmieszniejsze aby polepszyć smak i odczucie konsumenta w napoju znajdują się różne cukry i słodziki, które spowodują wyrzut insuliny, która z kolei zablokuje tłuszcz w komórkach... Wpieprzamy w dobrej wierze taką buteleczkę słodzików i później niestety trzeba robić refaktoring organizmu.
Ale wracając do kodu
Badanie złożoności poprzez metryki
Kwestia z metrykami jest nieco skomplikowana bo mogą być one pomocne ale niestety często są wykorzystywane do oceny ludzi co całkowicie je dyskwalifikuje. Jeśli nie użyjemy jej jako targetu czy innego gówna to może pomóc bo zobaczmy. Jeśli np. ktoś mi powie "będzie do przeniesienia 100kg cementu" to mniej więcej czuję o jaką ilość i materiału i pracy tu chodzi. Jeśli zaś ktoś powie "musimy dodać logowanie do kodu o złożoności cyklomatycznej 17" to ilu z was tak naprawdę "CZUJE" z czym ma do czynienia. Dodatkowo - ilu z was wie jak się doprowadza kod do złożoności cyklomatycznej 17? No właśnie. Dlatego dzisiaj zerknijmy na tę metrykę.
Jest sobie kod będący przepisem na wyciągnięcie pieniędzy z bankomatu.
public void wyciągnijPieniadzeZBanku(){
ubierzSie();
WezKarte();
IdzDoBankomatu();
UzyjKarty();
SchowajKase();
WracajDoDomu();
}
Do obliczenia złożoności cyklomatycznej wykorzystamy plugin "metrics" dla eclipse co w praktyce wygląda tak:
Dlaczego wartość tej metryki wynosi 1? Złożoność cyklomatyczna to tak naprawdę odpowiedź na pytanie "W ilu miejscach kod może zmienić swój bieg"
public void wyciągnijPieniadzeZBanku(){
if(padaDeszcz){
wezParasol();
}
ubierzSie();
WezKarte();
IdzDoBankomatu();
UzyjKarty();
SchowajKase();
WracajDoDomu();
}
Możemy wziąć parasol ale nie musimy - są dwie drogi wykonania metody - złożoność cyklomatyczna wzrosła do 2. Czy kod stał się mniej czytelny? Cóż to jest indywidualna ocena każdego czytelnika. Dla mnie nadal jest czytelny. Co do czytelności to tutaj jest jeszcze jedna kwestia. Zerknijmy na dwa kawałki kodu
public void wyciągnijPieniadzeZBanku(){
if(padaDeszcz){
wezParasol();
}
ubierzSie();
WezKarte();
IdzDoBankomatu();
UzyjKarty();
SchowajKase();
if(portfel.isEmpty()){
WracajDoDomu();
}else{
IdzZKolegamiNaBrowara();
}
}
oraz
public void wyciągnijPieniadzeZBanku(){
if(padaDeszcz){
wezParasol();
}
ubierzSie();
WezKarte();
IdzDoBankomatu();
UzyjKarty();
for (int i = 0; i < banknoty.size(); i++) {
portfel.schowaj(banknoty.get(i));
}
WracajDoDomu();
}
Obydwa kawałki kodu maja złożoność cyklomatyczną na poziomie 3 chociaż można się spierać czy oba są czytelne/nieczytelne w tym samym stopniu. Dlatego też nie ma się co spinać, że sama złożoność na danym poziomi dostarcza nam 100% potrzebnych informacji. Druga kwestia. Można by zastosować w kodzie pętlę "ForEach" ale niestety plugin metrics liczy tę złożność cyklometryczną zliczając ilość warunków logicznych dlatego zupełnie ignoruje konstrukcję "foreach" - sonar dla odmiany dał sobie radę.
public void wyciągnijPieniadzeZBanku(){
if(padaDeszcz){
wezParasol();
}
ubierzSie();
WezKarte();
IdzDoBankomatu();
UzyjKarty();
for (int i = 0; i < banknoty.size(); i++) {
portfel.schowaj(banknoty.get(i));
}
if(portfel.isEmpty() || nieChleje){
WracajDoDomu();
}else{
IdzZKolegamiNaBrowara();
}
}
Tutaj dodaliśmy jeszcze jeden warunek logiczny (niechleje) i Cyclomatic Complexity wynosi już 5. Pomału zaczyna się robić bajzel.Ile mamy teraz puntów decyzyjnych?
- Możemy brać parasol albo i nie
- możemy schować pieniądze do portfela albo i nie
- Jak nie mam kasy w portfelu to idę do domu
- Ale nawet jak mam kasę i nie chleję to i tak idę do domu
public void wyciągnijPieniadzeZBanku(){
if(padaDeszcz){
wezParasol();
}
ubierzSie();
if(jestZimno){
ZalozDodatkowoSportowyDres();
}
WezKarte();
if(golf.jestZatankowany()){
podjedzDoBanku();
}else{
IdzDoBankomatu();
}
if(pin==null){
pin=telefon.dzwonDo(ZIOMALE.WIESIEK);
}
banknoty=UzyjKarty(pin);
for (int i = 0; i < banknoty.size(); i++) {
portfel.schowaj(banknoty.get(i));
}
if(portfel.jestPusty() || nieChleje){
WracajDoDomu();
}else{
IdzZKolegamiNaBrowara();
}
Złożoność cyklomatyczna tego kawałeczka wynosi już 8. Sądzę, że zdolności percepcyjne większości czytelników są na takim poziomie, że każdy dostrzeże w tym kodzie naturę gówna. I na koniec ulubiona praktyka w pewnych kręgach czyli arrow code.
public void wyciągnijPieniadzeZBanku(){
if(padaDeszcz){
wezParasol();
}
ubierzSie();
if(jestZimno){
ZalozDodatkowoSportowyDres();
}
WezKarte();
if(golf.jestZatankowany()){
podjedzDoBanku();
}else{
IdzDoBankomatu();
}
if(pin==null){
pin=telefon.dzwonDo(ZIOMALE.WIESIEK);
}
banknoty=UzyjKarty(pin);
for (int i = 0; i < banknoty.size(); i++) {
portfel.schowaj(banknoty.get(i));
}
if(portfel.jestPusty() || nieChleje){
WracajDoDomu();
}else{
while(telefon.mamSrodki()){
for (ZIOMALE ziomal: ZIOMALE.values()) {
if(telefon.dzwonDo(ziomal)!=null){
if(zabkaJEstOtwarta()){
IdzZKolegamiNaBrowara();
}
}
}
}
}
Plugin Metrics określi wartość złożoności cyklometrycznej na 9 zaś sonar na 12. Jest to dowód na to, że nie ma co się co przywiązywać do konkretnych wartości tej metryki. Generalnie to co pokazuje sonar bardziej do mnie przemawia gdyż ten "arrow code" dosyć tutaj namieszał.
Teraz zostaje nam już tylko to gówno refaktorować. Na szybko może wyjść coś takiego :
public void wyciągnijPieniadzeZBanku(){
przygotujSie(padaDeszcz,jestZimno);
WezKarte();
SrodekTransportu srodekTransportu=uzyskajSrodekTransportu();
srodekTransportu.udajSieDoBankomatu();
pin=znajdzPin();
banknoty=UzyjKarty(pin);
portfel.schowaj(banknoty);
OpcjaNaWieczor opcjaNaWieczor=zbadajMojeOpcjeNaWieczor(portfel);
opcjaNaWieczor.zrealizuj();
Złożoność cyklomatyczna tej metody wynosi znowu 1. Mamy również chyba jeden spójny poziom abstrakcji. Niektóre koncepcje jak Telefon czy Ziomal zostały przykryte przez bardziej abstrakcyjne twory jak OpcjaNaWieczor. Gdzie się podziała złożoność? Nie zniknęła ale została równomiernie rozłożona a powstałe konstrukcje obiektowe mogą (lub nie) zwiększy "reużywalność" kodu i zmniejszyć koszt utrzymania.
Podsumowując: Im większa wartość złożoności cyklometrycznej (które lepiej brzmi z angielska Cyclomatic complexity) tym gorzej ale z drugiej strony konkretne wartość nie mówi nam wszystkiego i jest zależna od narzędzia. W kolejnych odcinach przedstawię naturę innych metryk. Mam nadzieję, że ten sposób tłumaczenia był bardziej efektywny niż pisanie wzoru na CC "M = E − N + 2P" i nurkowanie w teorię grafów (której sam do końca nie rozumiem"
Dodatkowe źródła :
1. Ciekawy wpis, dzięki!
OdpowiedzUsuń2. Bardzo celna uwaga o spójnym poziomie abstrakcji. Jeśli metoda finalizujTransakcję() zapisuje dane w bazie i wysyła maila, powinna ona tylko i wyłącznie wołać zapiszWbazie() i wyślijMaila(). Wywołanie tylko jednej z nich i niskopoziomowe API (JavaMail lub JDBC) pochodzącej z drugiej psuje percepcję i utrudnia czytania. A przy okazji obie takie metody z pewnością powinny trafić do innych klas.
3. Poglądowo cylcomatic complexity można tłumaczyć jako minimalna ilość testów jakie dana metoda wymaga do pełnego pokrycia.
Spoko, dzięki za dobre słowa!
OdpowiedzUsuńCo do poziomów abstrakcji to ponad dwa lata temu przy okazji rozkminy tego tematu dodałem wpis o poziomach abstrakcji. Może kogoś zainteresuje :
http://pawelwlodarski.blogspot.com/2011/02/poziomy-abstrakcji.html
PS.Generalnie takie czytanie samego siebie sprzed kilku lat to fajne doświadczenie. Aż czuję taki sztuczny formalizm oraz bezosobowość narratorska, które chyba były wynikiem wynikiem fasad obronnych otaczających psychikę. Generalnie to zajebiste uczucie przeżywać rozwój ;)