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
}
}
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
Brak komentarzy:
Prześlij komentarz