niedziela, 30 czerwca 2013

Jak rodzi się złożoność kodu - księga trzecia

Kiedy rozpoczynaliśmy pracę nad obecnym projektem mieliśmy już trochę doświadczeń i przemyśleń odnośnie tego co czyni kod niezwykle nieczytelnym i trudnym w rozwoju. Wśród głównych przyczyn leżały ogromne klasy opakowujące kod proceduralny, długie metody czy masa zagnieżdżonych warunków - a wszystko przypieczętowane zmiennymi o słodko brzmiących nazwach temp czy zzz.

Na tym polu odnieśliśmy zauważalny sukces gdyż nasza największa klasa ma chyba 400 linii, żadna metoda nie przekracza 30 linii a 90% kodu nie ma więcej niż jednego poziomu zagnieżdżenia. Klęskę ponieśliśmy za to na innym froncie, którego istnienia kilka lat tamu nawet się nie domyślałem. Otóż kiedy tworzyliśmy nasze małe czytelne klasy zawsze musieliśmy je gdzieś umieścić. DAO szło zazwyczaj do paczki costam.dao kontrolery do paczki costam.controlers itd.

Istnieje specjalna metryka w sonarze, która bada tego typu spierdoliny : I chociaż nie mam jeszcze w pełni intuicyjnego wyczucia tej metryki to wskazane wartości pokazują, że może być lepiej.

A co da nam to, że paczki nie będą tak poprzeplatane? Aby to zrozumieć złamiemy (ale tylko pozornie) jedną z zasad dobrego kodu obiektowego

Jak OOP tworzy Klasę Blob/Good

Klasa Money będzie dobrym przykładem bo w dzisiejszych czasach każdy potrzebuje pieniędzy. Na początek zobaczmy co się stanie kiedy przedstawimy pieniądze jako zwykłą strukturę danych:

 

public class Money {

    private final BigDecimal amount;
    
    private final Currency currency;

    public Money(BigDecimal bigDecimal, Currency currency) {
        this.amount = bigDecimal;
        this.currency = currency;
    }

    public BigDecimal getAmount() {
        return amount;
    }

    public Currency getCurrency() {
        return currency;
    }
}

Niestety powyższa forma jest zaproszeniem do tworzenia klas typu Utils czy Helper. A pod tym linkiem ----> http://nemo.sonarsource.org/ możecie zobaczyć, że klasy utils to jedne z bardziej złożonych tworów.

No to spróbujmy, żeby było bardziej obiektowo :

 

public class Money {
    private final BigDecimal amount;

    private final Currency currency;

    public Money(BigDecimal bigDecimal, Currency currency) {
        this.amount = bigDecimal;
        this.currency = currency;
    }
    
    public Money add(Money moneyToAdd){
        checkTheSameCurrency(moneyToAdd);
        return new Money(moneyToAdd.amount.add(this.amount),currency);
    }
    
    public boolean isGreaterThan(Money moneyToCompare){
        checkTheSameCurrency(moneyToCompare);
        return this.amount.doubleValue()> moneyToCompare.amount.doubleValue();
    }

    private void checkTheSameCurrency(Money moneyToAdd) {
        if(moneyToAdd.currency!=this.currency){
            throw new RuntimeException("zle");
        }
    }
}

Wydaje się być ok. Jest enkapsulacja, nie zdradzamy wewnętrznej implementacji, jest "tell don't ask" i inne takie dobre praktyki.

A co gdy domena się rozwija i muszę obliczyć podatek? "Tell don't ask" ?

 

public class Money {

    private final BigDecimal amount;

    private final Currency currency;

    public Money(BigDecimal bigDecimal, Currency currency) {
        this.amount = bigDecimal;
        this.currency = currency;
    }
    
    //!!!!!!
    public Tax calculateTax(double percentage){
        return new Tax(amount.multiply(BigDecimal.valueOf(percentage)));
    }

No i pytanie co tak naprawdę symbolizuje klasa Money? Jeśli reprezentuje pieniądz to polityka podatkowa średnio tam pasuje. Oczywiście można powiedzieć - "no to dodajmy tam operacje mnożenia czy też wyciągania procentu". Tak a za chwilę dodamy operacji całki i obliczania pola. A co jeśli będę chciał przeliczać waluty - czy w ramach enkapsulacji dodać funkcjonalność kantoru do klasy Money albo Currency?

Jeśli mówić o smrodzie w kodzie to powyższe opcje dla mnie niezwykle śmierdzą. A co dyby tak wykorzystać modyfikator dostępu, którego nie ma?

 

public class Money {

    private final BigDecimal amount;

    private final Currency currency;

    public Money(BigDecimal bigDecimal, Currency currency) {
        this.amount = bigDecimal;
        this.currency = currency;
    }
    
    BigDecimal getAmount() {
        return amount;
    }

...
}
public class TaxCalculator {

    private static final double standardTaxPercentage=0.23;
    
    public Tax calculateTax(Money money){
        return new  Tax(money.getAmount().multiply(BigDecimal.valueOf(standardTaxPercentage)));

    }
}
Dla wielu osób wynik tego niezwykle wciągającego eksperymentu myślowego może być czymś oczywistym ale ja na swojej drodze napotykam wiele osób, które potrzebują takiej wiedzy (w tym mnie). Ładujemy Money i TaxKalkulator do jednej paczki i mamy enkapsulację pieniędzy w ramach tej paczki. Powinno nam to pomoc w uniknięciu problemu klasy BLOBa ale co jeśli owa paczka niezwykle nam się rozrośnie? Tutaj trudno mi określić granicę kiedy "jest już za dużo".

Bonus - skąd się bierze fundamentalizm obiektowy

Znam pewnych ludzi, którzy kłócili by się, że powyższy przykład można rozwiązać jakimś wizytatorem czy innymi cudami. Problem polega na tym, że czasem mogą mieć rację a czasem nie ale sam fakt uznawania zasady rodem z wikipedii typu "tell don't ask" czy "DRY" za święte i nienaruszalne doprowadza mnie do szalonego płaczu. A w ogólności dobija mnie traktowanie OOP jako panaceum na wszystko - czyli zarzuty typu "ten kod jest zły bo nie obiektowy".

Jeśli ktoś ma niech zerknie sobie do rozdziału nr 6 "Czystego kodu" pod tytułem "struktury danych". Są tam dwa przykłady obliczania pól figur - jeden obiektowy a drugi proceduralny. Nie chce mi się kopiować tutaj źródeł ale wniosek postawiony przez autora jest jasny: wersja bardziej proceduralna z ifami jest lepsza jeśli dojdą nam nowe funkcje (jedno miejsce do zmiany vs wszystkie klasy figur do zmiany) - ale wersja z polimorfizmem jest lepsza jeśli spodziewamy się nowych kształtów - wniosek: obiektowy nie znaczy najlepszy.

Bo nawet czy jest sens toczyć spory odnośnie tego czy kod z ogromną klasą ale idealna enkapsulacją jest bardziej obiektowy niż kod z małymi klasami, które jednak nie starają się robić wszystkiego jednocześnie udostępniając gdzieniegdzie szczegóły implementacyjne?

A może jednak przychodzi czas aby nie ograniczać się do racjonalizacji swoich wyborów prostymi zasadami z wikipedii ale zgłębić dokładniej jakie te wybory niosą konsekwencje? Są różne modele rozwoju zdolności : Shu Ha Ri , SU HA RY , SŁU CHAJ STARY - opisujące jak to od prostych reguł należy przechodzić do zrozumienia zasad poprzez magiczną transcendencję. Na samym początku jest bagno, totalnie zjebany kod i tutaj jest miejsce na stosowanie zasad "Tell don't ask" czy "DRY" bez myślenia bo cokolwiek się nie zrobi to będzie lepiej. Ale jeśli zostanie się na tym etapie to jesteśmy na prostej ścieżce do Obiektowego Fundamentalizmu

O a teraz złamię jeszcze jedną zasadę - święte DRY

 

class PracownikUczelni{
private String tytuł;
}

class Film{
private String tytuł;
}

No i DRY jest złamane. Można oczywiście zrobić też tak:
 

class abstract PosiadaczTytułu{
private String tytuł;
} 

class PracownikUczelni extends PosiadaczTytułu{}

class Film extends PosiadaczTytułu{}

No i DRY jest spełnione a przy okazji mamy zjebaną hierarchię klas. NA początek zasady z wikipedii są dobre ale później trzeba zacząć używać mózgu - nie ma wyjścia. A jeśli ktoś potrzebuje potwierdzenia od jakiejś gwiazdy światowego kalibru to Dan North w jednej ze swoich prezentacji napisał "DRY is an enemy of decoupled" (A jeśli dla kogoś Dan North nie jest gwiazdą światowego kalibru albo zastanawia się "kto to k***a jest?" to niech delektuje się moimi wnioskami)

niedziela, 9 czerwca 2013

Jak rodzi się złożoność kodu - sezon drugi

W poprzednim odcinku opowiadaliśmy sobie o złożoności cyklomatycznej i również w tamtej części znajduje się rozbudowany wstęp epicko opowiadający o tym dlaczego należy się interesować jakością kodu i takie tam. Aby nie tracić teraz miejsca i czasu idziemy prosto do kolejnych metryk.

Spójność klasy - LCOM i LCOM4

Gdy po raz pierwszy zobaczyłem metrykę LCOM4 w sonarze do głowy przyszło mi, że to musi mierzyć ilość materiałów wybuchowych w kodzie albo coś takiego. W zasadzie to wysokie wskazania tej metryki mogą objawiać nieuchronną eksplozję projektu gdyż mierzy ona jak spójne są nasze klasy. Niech kod przemówi ...

ProductManager

Manager, Helper, Procesor - podobno to są skróty od zdania "nie mam pojęcia co dokładnie ma robić ta klasa i dlatego nie wiem jak ją nazwać". Niestety widziałem na własne oczy jak takie klasy rozrastają się do ponad 10000 linii bo jak coś nazywa się "Manager" to w zasadzie wszystko tam pasuje.
Na początek mamy :

 

public class ProductManager {
private List products = newArrayList();

 public List showProducts() {
  List currentlyAvailableProducts = newArrayList();
  for (Product product : products) {
   addProductIfAvailable(currentlyAvailableProducts, product);
  }
  return currentlyAvailableProducts;
 }

 private void addProductIfAvailable(
   List currentlyAvailableProducts, Product product) {
  if (product.isAvailable()) {
   currentlyAvailableProducts.add(product);
  }
 }
}

Taka sobie klasa opakowująca listę produktów. Na razie poza nazwą nie widać innych niebezpieczeństw. Zobaczmy co wskazują metryki :

  • (Metrics plugin) LCOM - 0
  • (Sonar) LCOM4 - 1

Nadszedł czas przyjrzeć się co to są te LCOM i LCOM4. Pierwsza - LCOM - jest dosyć dyskusyjna za co została podobno dosyć skrytykowana - ale o tym za chwilę. LCOM weźmie każdą parę metod i sprawdzi czy chociaż jedna z danych na których one operują jest wspólna. Na razie mamy dwie metody jedną publiczną jedną prywatna ale obydwie działają na produktach. Wynik zero oznacza, że nie ma żadnych metod rozłącznych działających na różnych danych czyli jest zajebiście.

LCOM4 z drugiej strony sprawdza czy wszystkie metody i dane są połączone strzałkami tak jak na rysunku poniżej (fachowo to się nazywa ilość niezależnych grafów).

Na razie wszystko jest połaczone czyli wartości metryki to 1 czyli jest zajebiście.

Wiele odpowiedzialności

Dodajmy teraz nową odpowiedzialność do klasy : obliczanie cen produktów ( w końcu to product manager)

 
public class ProductManager {

 private List products = newArrayList();


 private Set vipUsers=newHashSet();
 
 private BigDecimal vat=new BigDecimal("0.23");

 
 public List showProducts() {
  List currentlyAvailableProducts = newArrayList();
  for (Product product : products) {
   addProductIfAvailable(currentlyAvailableProducts, product);
  }
  return currentlyAvailableProducts;
 }

 private void addProductIfAvailable(
   List currentlyAvailableProducts, Product product) {
  if (product.isAvailable()) {
   currentlyAvailableProducts.add(product);
  }
 }
 
 public BigDecimal calculateGrossPrice(Product product,User user){
  if(vipUsers.contains(user)){
   return product.getNetPrice();
  }
  
  BigDecimal netPrice = product.getNetPrice();
  return netPrice.add(netPrice.multiply(vat));
 }

}

  • (Metrics plugin) LCOM - 1
  • (Sonar) LCOM4 - 2


Ponieważ metody obliczania ceny brutto oraz wyświetlania produktów korzystają z zupełnie innych zestawów danych, więc na rysunku dostaniemy dwa rozłączne grafy - co zwyczajnie oznacza, że możemy te funkcjonalności spokojnie rozdzielić.

Problemy z LCOM4 i Sonarem

Pierwsza wersja kalsy z LCOM4=2 korzystała tylko z pola vat i wyglądała mniej więcej tak :

public BigDecimal calculateGrossPrice(Product product,User user){
BigDecimal netPrice = product.getNetPrice();
return netPrice.add(netPrice.multiply(vat));
}

Pomimo, że w teorii powinno to wystarczyć aby podbić LCOM4 do dwójki to jednak sonar z premedytacją zlewał tę metodę. Po małych poszukiwaniach okazało się, że problem nie jest trywialny gdyż czasami metody takie jak gettery, toString() czy jakieś dziedziczenia narzucone przez frameworki sztucznie podbijają LCOM4. Programiście Sonara wyłączają samotne metody jednak jak się okazuje trochę tutaj przekombinowali w druga stronę i czasami takie małe niezależne metody nie są brane pod uwagę przy obliczaniu LCOM4. Więcej w ticketach :
* SONARJAVA-65
* SONAR-2941
* SONAR-2934
* dyskusja - " A getter/setter is defined as method which accesses a single field" - właśnie to miało chyba miejsce w powyższym przypadku, nawet jak technicznie nie był to getter ale taka metodka utilsowa.

A wracając do wartości LCOM=1 w pluginie metrics. Generalnie wartosć 1 to z tego co zrozumiałem najgorsza wartość jaką możemy uzyskać gdyż zakres dopuszczalnych wartości to [0,1]. Oznacza to ni mniej ni więcej jak to, że możemy dalej psuć klasę a wartość LCOM będzie cały czas wynosić 1. Chyba stąd ta cała krytyka. LCOM4 będzie się zwiększać razem z kolejnymi nadmiarowymi funkcjonalnościami (minus optymalizacje sonara)

Rozwiązanie

Tutaj wystarczy jakoś rozsądnie podzielić managera na odrębne kalsy. Np. część finansowa może pójść do :
 
public class FinancialCalculator {

 private Set vipUsers = newHashSet();

 private BigDecimal vat = new BigDecimal("0.23");
 
 
 public BigDecimal calculateGrossPrice(Product product,User user){
  if(vipUsers.contains(user)){
   return product.getNetPrice();
  }
  
  BigDecimal netPrice = product.getNetPrice();
  return netPrice.add(netPrice.multiply(vat));
 }
}


No i teraz trzeba wykazać się niesamowitymi pokładami braku inteligencji aby do takiej klasy wrzucić metodę "showProducts" gdzie do "Managera" czy "Helpera" nawet by pasowała bo tam wszystko pasuje.

RFC - Response For Class

Problem z tą metryką jest taki, iż trudno stwierdzić czy dana wartość to już za dużo czy jeszcze nie. Te wskazania traktować należy jako sygnał, że coś tutaj może być nie tak.

Wpłynąć można na nią raczej dosyć prosto - redukując liczbę wywołań innych klas ale także redukując liczbę wywołań siebie przez inne klasy. I już na początek niespodzianka. Poniższa pusta klasa ma RFC=2. Według dokumentacji pierwsze "+1" Bierze się z okazji domyślnego konstruktora. Skąd się bierze +2 - pojęcia nie mam - widocznie poza domyślnym konstruktorem sonar znajduje tam coś jeszcze.

 
public class BlogCreator {
}

Po dorzuceniu węgla :

 

public class BlogCreator {

 private Browser browser;
 private Keyboard keyboard;

 public void writeBlog() { +1
  browser.loginToBlogger();  +1
  Topic postTopic = workOnTopic();
  Post typedPost = keyboard.type(postTopic); +1
  review(typedPost); 
  browser.submit(typedPost); +1
 }

 private Topic workOnTopic() { +1
  return new Topic(); +1
 }

 private void review(Post typedPost) { +1
  //coś tam
 }
}

Mamy RFC=9. Powtórzonych wywołań się nie liczy czyli jakbym użył raz jeszcze browser.loginToBlogger() to RFC byłoby stałe. Podobnie tylko raz liczy się metody wewnątrz klasy czyli workOnTopic() jest zliczane przy deklaracji i można to sobie wywoływać ile razy tam się chce.

Czy RFC=9 jest duże? Kod powyżej jest raczej czytelny i paradoksalnie wrzucenie wszystkiego w jedną wielką metodę mogłoby zmniejszyć jednocześnie RFC jak i czytelność kodu (mniej metod ~ mniejsze RFC). Także osobiście polecałbym traktować tę metrykę jako taki wskaźnik pomocniczy a nie wartość absolutną.

Jak można zmniejszyć RFC zwiększając czytelność kodu? Trzeba pobawić się poziomami abstrakcji i przykryć koncepcje bardzo szczegółowe czymś bardziej ogólnym :

 
public class BlogCreator2 {

 private Medium internetMedium;
 
 public void writeBlog() {
  MentalCreation ideaForBlog=awakeDivineSparkAndTrascendenOverYourHumanLimitation();
  internetMedium.publishForHumanity(ideaForBlog);
 }

 private MentalCreation awakeDivineSparkAndTrascendenOverYourHumanLimitation() {
  return new MentalCreation();
 }
}

W tym przypadku się okazało się, że RFC spadło do 6. Dobrze? Niedobrze? Nie wiadomo. Generalnie wiele zależy tutaj od percepcji tych, którzy będą kod czytać. Generalnie po wpisaniu słowa "abstraction" w Amazonie na pierwszych miejscach wyskakują pozycje o sztuce, malarstwie i psychologii. Chyba jednak pewne rzeczy łatwiej będzie ogarnąć na polu matematyki i programowania funkcyjnego.

Z drugiej strony jak gdzieś w projekcie RFC wyskoczy na poziomie 50 to jednak coś może być mocno zjebanego. ProductManager z tego odcinka ma RFC=17. FinancialCalculator już tylko RFC=9; ProductProvider RFC=10 (zrodla tego nie wklejałem ale to jest reszta, która została w ProductManagerze po ekstrakcji FinancialKalkultora). Czyli każdą z tych klas teoretycznie łatwiej przeczytać ale razem mają RFC większe od RFC samego ProductManagera.

Klasa CyclomaticExample z poprzedniego odcinka w swojej najbardziej zjebanej formie ma RFC=21. Po refaktoringu RFC=11. Czyli w jakimś tam stopniu można założyć, że im RFC mniejsze tym czytelniej

Materiały

niedziela, 2 czerwca 2013

Jak rodzi się złożoność kodu (part łan)

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
Jeśli chcielibyśmy przetestować wszystkie scenariusze to też ich tam chyba jest już z kilkanaście. No to dorzućmy coś jeszcze :

 
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. http://en.wikipedia.org/wiki/Cyclomatic_complexity
  2. http://www.guru99.com/cyclomatic-complexity.html