Dziwne rozwiązania, które położą każdy projekt

W tekście przedstawię kilka nieprawidłowych rozwiązań, które w trakcie mojej długiej pracy widziałem w różnych projektach. Wszystkie developerskie błędy w rzeczywistych projektach związane są z Javą, ale mogą dotyczyć także innych języków i platform. Ostatnie problemy są natury organizacyjnej, ale - moim zdaniem - są równie ważne, jak te techniczne.

Od wielu lat wspieram zespoły w rozwijaniu dobrych praktyk oraz podnoszeniu jakości produktów. Obejrzałem wiele projektów, które były tworzone w sposób godny naśladowania. Widziałem takie, które wymagały nieco uwagi, a także te, które wymagały drastycznych kroków, żeby wrócić na prostą. W poprzednim tekście pisałem o tym, jak się przygotować na przyjęcie pomocy z zewnątrz, w postaci konsultanta. W tej i kolejnej części chciałbym pokazać, co zobaczyłem po przybyciu.

W tym artykule, który będzie pierwszą z dwóch części, skupimy się na dość technicznych aspektach, czyli cztery błędy, które sprawiają, że projekt szybuje wprost do śmietnika (nawet, jeśli chwilowo wydaje się, że jest sukcesem).

1. Niewłaściwe zastosowanie elementów języka na przykładzie enuma w Javie
2. Niepotrzebne grupowanie wielu funkcjonalności w jednej klasie
3. Architektura projektu, która nie dostosowuje się do jego rozmiaru
4. Implementowanie w ramach projektu ogólnie dostępnych bibliotek, frameworków, czyli ,,wymyślanie koła”

Przejdźmy do przykładów. Celowo nie nazywam ich antywzorcami, bo do pewnego momentu, jeśli coś działa to działa i nie ma o czym dyskutować. Trzeba tylko pamiętać, że niektóre rozwiązania w pewnych kontekstach się sprawdzają, a w innych stanowią problem.

Niewłaściwe zastosowanie elementów języka na przykładzie enuma w Javie

Na początek zadajmy sobie pytanie: w jakim celu w języku pojawiły się enumy? Kilkanaście dni temu podczas kursu programowania w Javie moi „studenci" bardzo szybko wychwycili, że enum to typ jak każdy inny, ale ze skończoną liczbą wartości. Jeśli zdefiniujemy w programie typ User, to możemy tworzyć dowolną liczbę instancji. Zwykle ma to sens. Czasami jednak potrzebujemy ich skończonej liczby. Dla przykładu kierunki (lewy, prawy, góra, dół, przód, tył):

enum Direction {
  LEFT, RIGHT,
  TOP, DOWN,
  FORWARD, BACKWARD;
}

W związku z tym, że w Javie enumy to właściwie klasy, możemy dać im nieco więcej funkcjonalności. W jednym z projektów, do którego na chwilę dołączyłem, zastałem wzbogacone enumy. Jeden z nich wyglądał podobnie do tego poniżej:

public enum XDiagnosticStatus {
    VAL1("xxx"),
    VAL2("yyy"),
    VAL3("zzz");

    private String value;
    private String description;
    
    XDiagnosticStatus(String value) {
        this.value = value;
    }
    
    public String getValue() { return value; }
    private void setValue(String value) { this.value = value; }

    public String getDescription() { return description; }
    public void setDescription(String description) { 
        this.description = description;
    }
}
​

Na pierwszy rzut oka wydało mi się dość dziwne pozostawienie w kodzie enuma metod typu set*. Wstępnie założyłem, że autor wygenerował je i pozostawił, ale naprawdę podejrzane wydało mi się, że konstruktor enuma jest jednoargumentowy. Hmm… Korzystając z IDE, wyszukałem wszystkie użycia metody setValue(...). Żadnych wyników. Super. Usunąłem więc tę metodę jako zbędną. Ponowiłem szukanie tym razem dla metody setDescription(...) i niestety trafiłem na użycie. Wyglądało mniej więcej tak (komentarze dodałem od siebie dla potrzeb tekstu):

…
XDiagnosticStatus result;

if (... coś tam ...) {
  // super!
} else if (...coś innego...) {
  // tu po raz pierwszy pojawia się dziwoląg
  result = XDiagnosticStatus.VAL1;
  result.setDescription("Jakiś szeroki opis pierwszego przypadku..." + xy); // nigdy nie rób tego w domu!
} else { // jest błąd, trzeba z tym coś zrobić
   // i tutaj po raz drugi pojawia się szczyt inżynierii oprogramowania.
   result = XDiagnosticStatus.VAL2;
   result.setDescription("Tutaj pojawia się obszerny opis drugiego przypadku..." + yz); // nigdy nie rób tego w domu!
}

return result;

Linia z komentarzem jest kluczowa. Dlaczego? Enum w Javie jest gwarantowanym przez JVM singletonem i często bywa używany w tej funkcji. Po szczegóły odsyłam do świetnej książki J. Blocha „Efektywne programowanie w języku Java".

O ile nie potępiam całkowicie podobnych rozwiązań (setXXX na enumie) i w naprawdę wyjątkowych wypadkach uznaje je za niezbędne, to użycie takiego obiektu do przenoszenia pomiędzy „warstwami" aplikacji informacji o błędzie w ten sposób jest co najmniej błędne. Chociaż nazwałbym to niebywale nieostrożnym, szczególnie w aplikacji www, działającej w wielowątkowym kontenerze JEE. Przykład:

• jeden z wątków kontenera natrafia na problem i wchodzi w gałąź else if, zapisuje referencję do VAL1 i po chwili uzupełnia opis, korzystając z dynamicznej zmiennej xy. W tej chwili wątek jest wstrzymywany (zabrakło rdzeni w procesorze :)) i sterowanie przechodzi do kolejnego, który tym samym kodem wchodzi w gałąź else. Tutaj metoda setDescription(...) wywoływana jest z kompletnie innym parametrem i... wątek jest wstrzymywany, a kontrola wraca do wątku poprzedniego
• pierwszy wątek, odzyskawszy kontrolę, ma w tym miejscu do zrobienia tylko jedną rzecz return result; i zwraca nieprawidłową informację

Efektem będzie koszmar programisty, który podczas debugowania tego kodu osiwieje i będąc bliskim rozpaczy, zdecyduje się zmienić pracę, albo… zagłębiwszy się w kod, natrafi na ten wytrysk programistycznego geniuszu i rzuci w kogoś klawiaturą :) Miejmy nadzieję, że obędzie się bez ofiar.

Niepotrzebne grupowanie wielu funkcjonalności w jednej klasie

Duża część z nas (wielokrotnie przekonałem się, że nie wszyscy) zna wzorzec DAO (kto nie zna, czym prędzej powinien to nadrobić). W skrócie, każda klasa modelu aplikacji, która wymaga zapisywania w DB, posiada własną klasę *DAO, która posiada metody odpowiedzialne za utrwalanie w zewnętrznym repozytorium - zwykle w bazie danych. Działa to mniej więcej tak:

User -> UserDAO
Product -> ProductDAO​

...dzięki czemu mamy stosunkowo małe klasy DAO (w zależności ilości obiektów dziedziny może być ich dużo). Tymczasem w jednym z projektów znalazłem ogromną klasę DBUtil, która właściwie była/jest klasą DAO dla wszystkiego, co w projekcie występuje. Wszystkie metody - dla wygody! - w takiej klasie są metodami statycznymi, za każdym razem nawiązywane jest połączenie z bazą danych (na szczęście najczęściej jest pobierane z puli połączeń, ale nie zawsze jest to tak oczywiste). W klasie są setki stringów zawierające pełne zapytania SQL. Dziesiątki lub setki razy wykonywana jest ta sama żmudna procedura uzupełniania obiektu PreparedStatement parametrami. Kod najczęściej wygląda podobnie jak ten poniższy:

private static void logSomething(String userLogin, boolean success) {
  try (CustomConnect db = new CustomConnect()) {
    UserDbData userDbData = getUserDbData(db, userLogin);
    String q = "INSERT INTO table_name(index,timestamp,user_login,success) VALUES (NULL,?,?,?)";
    try (PreparedStatement ps = db.prepareStatement(q)) {
      long now = System.currentTimeMillis();
      ps.setTimestamp(1, new Timestamp(operationStart));
      ps.setLong(2, userLogin);
      ps.setString(3, success);
      ps.executeUpdate();
    }
  } catch (SQLException e) {
    log.error("Exception executing query", e);
  }
}
​

Dlaczego jest to złe? A może nie jest? To zależy (odpowiedź rasowego konsultanta :)). Otóż, dopóki działa, kod jest niewielki, a klasa zawiera tylko kilka takich metod, to można zaakceptować nawet fakt, że są one statyczne. Opisane warunki sugerują, że pracujemy nad czymś w rodzaju PoC lub prototypu. Jeśli jednak przechodzimy do kodu, który ma działać na produkcji, to podobne praktyki są niedopuszczalne (widziałem niedawno w takiej superklasie, ponad stulinijkowy złożony kod SQL - jej autor pokazywał ten kod z dumą w głosie, której nie sposób było nie usłyszeć)! Statyczne metody i nawiązywanie połączenia w ich obrębie utrudnia (czyniąc niemalże niemożliwym) ich testowanie.  Dużo kodu, którego pisania można w ogóle uniknąć, stosując gotowe biblioteki jak EclipseLink, Hibernate, które są świetnie zintegrowane z javowym ekosystemem.

Najlepszym rozwiązaniem jest najpewniej użycie czegoś w rodzaju SpringDataJPA i całkowite uniknięcie pisania tego kodu. Mój mały proof of concept w tym projekcie zmniejszył tego potwora (blisko 4 tysiące linii kodu) do mniej niż 400 linii z wykorzystaniem SpringDataJPA, niestety nie zdążył wejść do głównej gałęzi rozwojowej.

REASUMUJĄC: Nigdy nie implementuj w kodzie produkcyjnym warstwy trwałości (ang. persistence layer) w postaci jednej superklasy! Unikaj implementowania DAO na niskim poziomie! O ile to możliwe, zastosuj dostępne i sprawdzone biblioteki oraz użyj wzorca znanego większości programistów - DAO.

Wyryta w kamieniu architektura, czyli monolit vs. moduły vs. mikrousługi

Projekty, które okazały się przydatne, z czasem się rozrastają. To jest moment, w którym należy przemyśleć strukturę oraz architekturę rozwiązania, która może (i powinna) ewoluować w cyklu życia aplikacji. Nie mam na myśli drastycznych zmian, ale często podczas implementowania nowych funkcjonalności, dostrzegamy (albo i nie), że pierwotnie zaprojektowana struktura lub architektura aplikacji przestaje do niej pasować. Po czym rozpoznać, że ten moment nadszedł? Czasami to kwestia smaku (a jakże!) innym razem wprost natrafiamy na dokuczliwe utrudnienia.

Bywa, że obok siebie w projekcie pojawiają się generowane z WSDL-i klasy, które zmieniają się niezwykle rzadko, ale wydłużają niepotrzebnie proces rekompilacji, innym razem okazuje się, że jakaś istotna część projektu znajduje się w oddzielnym repozytorium i wymaga cyklicznej aktualizacji. Sprawia to, że praca z projektem staje się niewygodna. Gdy się temu przyjrzymy, widzimy kilka rozwiązań struktury:

• monolityczna aplikacja w monolitycznej strukturze - jedno repozytorium, jeden katalog src/
• monolityczna aplikacja w modułowej strukturze - jedno repozytorium, wiele katalogów src/ lub po prostu wiele repozytoriów, jednak aplikacja ostatecznie deployowana jako całość
• architektura zorientowana na mikrousługi - wiele repozytoriów, każde z własnym katalogiem src/, z każdego z nich budowana oddzielnie deployowana aplikacja, wszystkie łącznie stanowią końcowe rozwiązanie

Każda z tych form ma zalety i wady, którym należy się przyjrzeć.

Monolityczna aplikacja w monolitycznej strukturze

Zalety:

• niewątpliwą zaletą jest prostota developmentu (do czasu, gdy stopień skomplikowania przekracza granice rozsądku), po ściągnięciu projektu z repozytorium uruchamiamy build (np. mvn clean run)
• łatwość przeszukiwania kodu
• aplikacja w takiej strukturze łatwo się poddaje wszelkim refaktoryzacjom - cały wytworzony przez nas kod dostępny jest "pod ręką"
• wdrażanie aplikacji "na produkcję" jest proste (w założeniu), bo wymaga wdrożenie jednego artefaktu

Wady:

• wraz ze wzrostem ilości kodu wydłuża się czas budowania
• przy dużych projektach zbudowanie w głowie modelu aplikacji wymaga sporego wysiłku
• mieszają się różne warstwy/komponenty aplikacji (GUI obok DAO, itd.)

Monolityczna aplikacji w modułowej strukturze

Zalety:

• dzieli zalety monolitycznej aplikacji w monolitycznej strukturze
• znacznie łatwiej budować model aplikacji w głowie (struktura modułów nieco ten model sugeruje)
• odpowiednia konfiguracja budowania znacznie skraca czas budowania w stosunku do "pełnego monolitu"

Wady:

• wciąż chcąc zmienić choćby drobną rzecz w projekcie, musimy deployować aplikację w całości

Architektura (i struktura) zorientowana na mikrousługi

Zalety:

• pojedyncze projekty mikrousług są małe i bardzo łatwo zbudować ich model w głowie
• błyskawiczna kompilacja
• poszczególne usługi mogą być tworzone w różnych technologiach, na dowolnym etapie można dopasować technologię do jej potrzeb

Wady:

• dopóki interfejsy usług nie ustabilizują się (z mojego doświadczenia wynika, że nie następuje to nigdy), refaktoryzacje mogą być dość kłopotliwe (patrz Spring Remote, RPC)
• wdrażanie zmian na produkcję może być koszmarem (chyba, że zostanie zautomatyzowane - patrz Ansible, Kubernetes itp.)

Oczywiście powyższa lista w żadnym wypadku nie wyczerpuje wszystkich kombinacji a listy wad i zalet na pewno nie są pełne. Myślę jednak, że dają jakiś pogląd na to, jakie decyzje musimy podejmować na każdym etapie pracy z projektem.

REASUMUJĄC: struktura i architektura projektu nie może być wyryta w kamieniu! Jeśli istnieje konieczność zmiany, to należy zmiany wdrożyć.

Implementowanie w ramach projektu ogólnie dostępnych bibliotek, frameworków (wymyślanie koła)

To chyba jeden z największych grzechów programistów. Szczególnie często popełniany przez juniorów z kilkuletnim doświadczeniem (tak, kilka lat pracy nie czyni z programisty seniora), którzy czują, że mogą napisać już dojrzały framework samodzielnie. I będzie zdecydowanie lepszy niż ten, który na rynku jest rozwijany przez setki programistów i używany przez tysiące :) To się zdarza.

Często zespoły składają się z grupy programistów, wśród których nie ma żadnego seniora. Wtedy pojawia się prawdziwy dramat. Cała grupa ma podobne spojrzenie na takie wyzwanie. Wszyscy zgadzają się, że to dobry pomysł. Efektem tego są zamknięte do jednej organizacji (często do jednego projektu, bo jakość rozwiązania jest tak marna, że nikt w organizacji nie chce już tego używać) frameworki MVC lub ORM. Przybierają one często zaskakująco oryginalne nazwy:

• NazwaOrganizacjiORM
• NazwaOrganizacjiCustomORM
• NazwaOrganizacjiSpecialMVC
• Itd…

Zastanawiam się, czy tego typu rozwiązania nie są gorsze, niż opisane wcześniej DAO w jednej klasie, ale chyba nie… to jednak jest tak samo złe :)

Jak uniknąć kopiowania?

Po czym poznać, że od nowa implementujesz coś, co zapewne jest dostępne? Jeśli w kodzie pojawia się specjalna konfiguracja do wstrzykiwania zależności i jest on obsługiwana przez inny kawałek kodu w projekcie, to jest to znak, że próbujesz napisać nową wersję Springa. Jeśli masz jakiś kod, który na podstawie konfiguracji automatyzuje warstwę trwałości… to znak, że piszesz nowego Hibernate’a. Podobnych przykładów można wymienić jeszcze kilka.

Ogólnie rzecz biorąc - jeśli korzystasz z API refleksji albo używasz XML-a, JSON, Yaml do czegoś innego niż serializacji obiektów biznesowych, to wiedz, że coś się dzieje.

REASUMUJĄC: Zanim napiszesz, kod zastanów się, czy nie został on już napisany, skup się na rozwiązywaniu problemów, a nie tworzeniu nowych.

Podsumowanie

Możesz w oczach „biznesu” być geniuszem informatyki, na ścianie twojego biura mogą wisieć dziesiątki dyplomów i pochwał, ale jeśli zaprojektujesz kiepską architekturę, to dla inżynierów będziesz po prostu projektantem słabej architektury i nie zmieni tego kolejna nagroda od prezesa.