Problem powtarzającej się walidacji — Driver License
Być może pamiętasz sytuację w swoim projekcie, gdy pracując nad wprowadzeniem drobnej zmiany, zaszła konieczność zmodyfikowania wielu podobnie wyglądających fragmentów kodu. Czasem tych miejsc jest niewiele, na przykład tylko dwa lub trzy, ale czasami są ich dziesiątki.
Oczywiście każde ze zmodyfikowanych miejsc pociąga za sobą potrzebę przetestowania, czy aby na pewno aplikacja nadal zachowuje się zgodnie z oczekiwaniami. Im większy jest zakres wprowadzanych zmian i im więcej klas będziemy modyfikować, tym częściej pojawia się strach czy na końcu wszystko będzie dobrze. Pomyłka może wynikać z wielu powodów — na przykład z twojego zmęczenia spowodowanego wielokrotnym powtarzaniem bardzo podobnie wyglądającego kodu.
W przypadku projektu Cabs aplikacja w krótkim czasie ma zostać wprowadzona na nowe rynki.
Wiele rzeczy wygląda i funkcjonuje tam w zupełnie inny sposób, przykładowo numery praw jazdy wyglądają zupełnie inaczej. Wymaga to stosowania zupełnie odmiennych reguł walidacyjnych — wynikają one bezpośrednio z lokalnych przepisów prawa. Zatem wraz z wchodzeniem na kolejne rynki należy modyfikować kod związany tą właśnie walidacją. Zmiana pozornie nieduża, jednak przez podejmowane wcześniej decyzje techniczne, jej wprowadzenie jest utrudnione i wymaga zwielokrotnionego wysiłku.
Spójrz na kod klasy DriverService:
public class DriverService {
public static final String DRIVER_LICENSE_REGEX =
"^[A-Z9]{5}\\d{6}[A-Z9]{2}\\d[A-Z]{2}$";
public Driver createDriver(String license, String lastName,
String firstName, Driver.Type type,
Driver.Status status, String photo) {
Driver driver = new Driver();
if (status.equals(Driver.Status.ACTIVE)) {
if (license == null || license.isEmpty() ||
!license.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalArgumentException("Illegal license no = " + license);
}
}
driver.setDriverLicense(license);
// ...
}
// ...
}
Widać w niej dokładnie problem opisany powyżej. Walidacja numeru prawa jazdy kierowcy występuje tu w wielu różnych przypadkach użycia, między innymi podczas dodawania kierowcy do systemu, zmiany jego statusu:
public class DriverService {
public static final String DRIVER_LICENSE_REGEX =
"^[A-Z9]{5}\\d{6}[A-Z9]{2}\\d[A-Z]{2}$";
// ...
public void changeDriverStatus(Long driverId, Driver.Status status) {
// ...
if (status.equals(Driver.Status.ACTIVE)) {
String license = driver.getDriverLicense();
if (license == null || license.isEmpty() ||
!license.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalStateException("Status cannot be ACTIVE.
Illegal license no = " + license);
}
}
// ...
driver.setStatus(status);
}
}
czy też zmiany samego numeru prawa jazdy:
public class DriverService {
public static final String DRIVER_LICENSE_REGEX =
"^[A-Z9]{5}\\d{6}[A-Z9]{2}\\d[A-Z]{2}$";
// ...
public void changeLicenseNumber(String newLicense, Long driverId) {
// ...
if (newLicense == null || newLicense.isEmpty() ||
!newLicense.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalArgumentException("Illegal new license no = " + newLicense);
}
// ...
driver.setDriverLicense(newLicense);
}
// ...
}
Właściwe przetestowanie z osobna każdego z tych zachowań w kontekście poprawności numeru prawa jazdy oznacza multiplikację przypadków testowych, a w konsekwencji kolejny dodatkowy kod do napisania i późniejszego utrzymania.
W tym przypadku nietrudno zauważyć występujące powtórzenia. Są one zlokalizowane w ramach jednej klasy, w dodatku w występujących kolejno po sobie metodach. Czasem jednak problem ten występuje w znacznie większej skali, powtórzenia są zlokalizowane w różnych klasach, a te z kolei w różnych częściach systemu. W takim przypadku pomocna będą:
- analiza historii repozytorium,
- statyczna analiza kodu.
Pozwolą one wychwycić miejsca, które zawsze są zmieniane razem. Takich narzędzi na rynku jest wiele i są one dopasowane do konkretnych technologii i języków programowania.
Oczywiście usuwanie powyższego problemu z kodu powinno wynikać z konkretnej potrzeby. Jeśli nie pojawiają się nowe wymagania biznesowe, dla których warto poprawić jakość kodu, a ostatnia jego modyfikacja pochodzi sprzed wielu tygodni czy wręcz miesięcy, prawdopodobnie byłaby to sztuka dla sztuki. Trudno będzie uzasadnić koszty wynikające z potrzeby prac, gdy na horyzoncie jest wiele innych zadań. Złamanie metody Don’t Repeat Yourself, która mówi o duplikacji logiki (niekoniecznie kodu), samo w sobie nie jest problemem. Problemem natomiast staje się wtedy, kiedy musimy ten kod za każdym razem zmieniać w wielu miejscach i o tym pamiętać.
W przypadku projektu Cabs problem ze strukturą projektu został zidentyfikowany i nazwany: logika walidacji powtarza się w wielu miejscach. Posługując się w dowolnym miejscu systemu prawem jazdy o typie String, nie mamy pewności co do jego poprawności. Zanim jednak przystąpimy do rozwiązań, należy zadbać o bezpieczeństwo wprowadzania zmian, aby był to proces bezstresowy.
❓ Jaki problem rozwiązujemy?
Duplikacji zmiennej walidacji.
Jak nasz problem rozwiązać?
Zastanów się nad możliwymi opcjami:
- Czy warto przenieść walidację do klasy Utils? Gdzie znalazłby się też inne tego typu funkcje.
- A może klasa DriverValidator będzie lepszym rozwiązaniem?
- Może po prostu zwykła metoda prywatna, validateDriverLicense w klasie DriverService?
- Być może podejdziemy do naszego problemu z innej strony, zamieniając typ String na typ DriverLicense, który enkapsuluje taką walidację?
Która opcja ma w kontekście projektu Cabs najwięcej zalet?
Skoro zmiany są częste i dodatkowo rozsiane po całym systemie, to zdroworozsądkowym rozwiązaniem jest ich "ściągnięcie" do jednego miejsca — klasy, komponentu, obiektu, do którego wszystkie miejsca w kodzie będą się odwoływać. W ten sposób przyszłe modyfikacje funkcji biznesowych związanych z poprawnością numeru prawa jazdy odbędą się w tym nowo powstałym bycie.
Klasa Utils
Spotykana w niejednym systemie, a w wielu z nich owiana sławą. W oderwaniu jednak od negatywnych emocji, po inżyniersku, zastanówmy się, czy dzięki niej nasz problem zostałby rozwiązany.
Klasa Utils może mieć przecież metodę validateLicense:
Wszystkie obecne miejsca mogą z niej skorzystać, przyszłe zmiany zamkną się właśnie w ciele tej metody. Warto się tu jednak zatrzymać.
❓ Jaki problem rozwiązujemy?
Duplikacji kodu.
❗ A jaki problem wprowadzamy?
Wszystkie obecne miejsca skorzystają z nowej metody. Możliwe, że jest ich całkiem sporo. Jest też duża szansa, że odpowiedzialność tej klasy nie kończy się na walidacji numeru prawa jazdy. Klasa Utils robi zwykle dużo innych rzeczy — najczęściej w żaden sposób ze sobą niezwiązanych. Oznacza to, że jej klientów jest znacznie więcej.
Tworzy się nam centralny byt, który jest narażony na ciągłe zmiany. Pojawiają się tzw. merge konflikty. Ciągłe zmiany kodu, który ma wielu klientów, to sytuacja, która niesie ze sobą ogromne ryzyko wprowadzenia błędu w losowej części systemu. Jeden, często zmieniany centralny byt, o którym wiedzę ma każdy zakamarek naszego systemu, chyba nie jest dobrym pomysłem.
Wiemy też, że w przyszłości pojawią się nowe sposoby walidacji numerów praw jazdy:
class Utils {
public static boolean validateLicensePL(String license) {
// ...
}
public static boolean validateLicenseUK(String license) {
// ...
}
public static boolean validateLicenseRU(String license) {
// ...
}
}
Poprawność polskiego prawa jazdy sprawdza się inaczej niż angielskiego czy rosyjskiego. Czy w takim razie powinniśmy mieć kilka osobnych metod dedykowanych dla poszczególnych krajów? W takim przypadku kod w każdy kliencie wywołującym te metody przybrałby następującą postać:
if (country.equals("PL")) {
result = utils.validateLicensePL(driverLicense);
} else if (country.equals("RU")) {
result = utils.validateLicenseRU(driverLicense);
} else if (country.equals("UK")) {
result = utils.validateLicenseUK(driverLicense);
}
Dodanie nowych krajów powoduje, że wracamy do początku problemu — ciągłe zmiany w instrukcjach warunkowych w wielu miejscach.
Kolejnym podejściem byłoby przekazanie do tej metody parametru określającego kraj i całą tę "ifologię" można by było umieścić w ciele metody:
W takim wypadku klasa Utils rośnie jeszcze bardziej. Istnieje też ryzyko, że nowa osoba w projekcie nie wie o tej klasie. Wprowadza więc swoją klasę Utils, a konsekwencje są tu już oczywiste.
Walidator lub prywatna metoda
Rozwiązaniem problemów klasy Utils mogłaby być klasa DriverLicenseUtils lub DriverLicenseValidator. Zajmowałaby się wyłącznie prawem jazdy, otrzymując informację o kraju w parametrze, wykonując swoją pracę i zwracając wynik w postaci Stringa.
❓ Jaki problem rozwiązujemy?
Duplikacji kodu.
❗ A jaki problem wprowadzamy?
Jak zwykle, diabeł tkwi w szczegółach. O jednym ze szczegółów zapomnieliśmy: ten reprezentujący numer prawa jazdy String może być rozsiany po wielu miejscach w systemie. Podobnie byłoby w przypadku prywatnej metody validateDriverLicense (opcji numer 3.).
Czy widząc jego instancję, mamy pewność, że jest poprawny? Może idąc tropem defensywnego programowania, będziemy to za każdym razem sprawdzać? Powoduje to automatycznie sporo nadmiarowego kodu. A co jeśli nowa osoba w projekcie zapomni tego zrobić? Jak zapewnić wywołanie naszego walidatora za każdym razem, kiedy mamy do czynienia z tym problematycznym prawem jazdy?
Klasa DriverLicense
Rozwiązaniem mogłaby być klasa DriverLicense, która w środku zawsze sprawdza poprawność numeru prawa jazdy:
public class DriverLicense {
public static final String DRIVER_LICENSE_REGEX =
"^[A-Z9]{5}\\d{6}[A-Z9]{2}\\d[A-Z]{2}$";
private String driverLicense;
public DriverLicense() {
}
public static DriverLicense withLicense(String driverLicense) {
if (driverLicense == null || driverLicense.isEmpty() ||
!driverLicense.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalArgumentException("Illegal license no = " + driverLicense);
}
return new DriverLicense(driverLicense);
}
public String asString() {
return driverLicense;
}
}
Jeśli w dowolnym miejscu systemu mamy styczność z instancją takiej klasy, mamy pewność, że jej stan jest poprawny — co powoduje większe poczucie bezpieczeństwa i mniej kodu. Przyszłe zmiany też mogą zamknąć się w obrębie tej klasy. Wydaje się, że jest to rozwiązanie optymalne, rozwiewa wszystkie wymienione przez nas wątpliwości.
❓ Jaki problem rozwiązaliśmy?
Duplikację kodu.
Zapewnienie bezpieczeństwa zmiany
Sposób na zapewnienie bezpiecznego wprowadzenia klasy DriverLicense przedstawiony jest na livecodingu: Lekcja 2.1.3. Zapewnienie bezpieczeństwa zmiany
Zmiana w repozytorium kodu:
Dodatkowa funkcja testów
Napisane testy, które działają na wysokości obserwowalnych zachowań i nie dotykają szczegółów systemu, dają nam pełne bezpieczeństwo zmiany tych szczegółów. Będzie to szczególnie widoczne w kolejnych krokach refaktoryzacji. Nie chodzi tu o to, żeby pisać tylko testy integracyjne. W projektach typu legacy często jest to jedyne wyjście, żeby móc testować wspominane zachowania. Jednak bezpieczeństwo to tylko jedna z wielu funkcji testów w systemach legacy.
Pisanie testów działających na obserwowalnych zachowaniach pozwala także:
- uczyć się systemu
- na obserwowanie reakcji systemu — pozwala zobaczyć reakcję systemu na pewne sygnały, rozkazy. Uruchamiając test, możemy zobaczyć, w jaki sposób system odpowiada albo nie odpowiada. Jak sygnalizuje błędy, jak można je ZAOBSERWOWAĆ.
- na naukę procesu i struktury komponentów projektu — pisanie i uruchamianie testów może nam wiele powiedzieć o tym, co system musi wykonać jako kroki wejściowe, żeby jakaś funkcja była dostępna. Na przykład próba uruchomienia obserwowalnego zachowania zakończenia przejazdu — funkcji completeTransit() zwraca nam wyjątek — krótka analiza pozwala zobaczyć, że najpierw przejazd powinien być w odpowiednim stanie. Uczymy się procesu biznesowego.
- dokumentować zrozumienie systemu — przeglądając projekt, który nie ma dokumentacji (na przykład na GitHubie), najczęściej zaczyna się od przeglądania testów, aby zobaczyć przykład użycia. I podobnie może być w projekcie. Dokumentujemy nasze zrozumienie systemu legacy.
Techniki te nie są nieomylne i stosując je, nie mamy pewności, że zapewnimy w 100% bezpieczeństwo systemu, przy okazji poznając każdy jego zakątek. Na pewno pomagają one jednak nam w krokowej poprawie stanu systemu. Zauważ, że jest to efektywniejsze od uruchamiania tego systemu na lokalnej maszynie po każdej zmianie. Przy okazji, praca z testami może być wykorzystana też przez innych członków zespołu. Czego nie można powiedzieć o lokalnym uruchamianiu i "przeklikiwaniu" zmian.
Wprowadzenie konceptu DriverLicense
Wprowadzenia klasy DriverLicense przedstawione jest na livecodingu: Lekcja 2.1.5. Wprowadzenie nowego konceptu
Zmiana w repozytorium kodu:
Podłączenie nowego kodu
Podłączenie klasy DriverLicense w miejscach, w których do tej pory operowaliśmy na Stringu opisującym numer prawa jazdy, przedstawione jest na livecodingu: Lekcja 2.1.6. Podłączenie nowego kodu
Zmiana w repozytorium kodu
Obserwacja efektów
Nadszedł moment, żeby sprawdzić, czy wprowadzone rozwiązanie, rozwiązuje wspomniany problem. Spójrz na aktualny kod:
public class DriverLicense {
public static final String DRIVER_LICENSE_REGEX =
"^[A-Z9]{5}\\d{6}[A-Z9]{2}\\d[A-Z]{2}$";
private String driverLicense;
public DriverLicense() {
}
private DriverLicense(String driverLicense) {
this.driverLicense = driverLicense;
}
public static DriverLicense withLicense(String driverLicense) {
if (driverLicense == null || driverLicense.isEmpty() ||
!driverLicense.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalArgumentException("Illegal license no = " + driverLicense);
}
return new DriverLicense(driverLicense);
}
public static DriverLicense withoutValidation(String driverLicense) {
return new DriverLicense(driverLicense);
}
public String asString() {
return driverLicense;
}
}
Teraz zmiana sposobu walidacji zamyka się w jednym miejscu. Kod tego miejsca nie musi być na razie najpiękniejszy, ponieważ ważne jest to, że jego poprawa oznacza zmianę jednej klasy, która posiada stabilny interfejs.
Wcześniej zmiany musiały odbywać się przynajmniej 3 miejscach:
public class DriverService {
public Driver createDriver(String license, String lastName,
String firstName, Driver.Type type,
Driver.Status status, String photo) {
// ...
if (status.equals(Driver.Status.ACTIVE)) {
if (license == null || license.isEmpty() ||
!license.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalArgumentException("Illegal license no = " + license);
}
}
// ...
}
public void changeLicenseNumber(String newLicense, Long driverId) {
// ...
if (newLicense == null || newLicense.isEmpty() ||
!newLicense.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalArgumentException("Illegal new license no = " + newLicense);
}
// ...
}
public void changeDriverStatus(Long driverId, Driver.Status status) {
// ...
if (status.equals(Driver.Status.ACTIVE)) {
String license = driver.getDriverLicense();
if (license == null || license.isEmpty() ||
!license.matches(DRIVER_LICENSE_REGEX)) {
throw new IllegalStateException("Illegal license no = " + license);
}
}
// ...
}
}
Kod innych miejsc, które korzystały z numeru prawa jazdy w postaci czystego Stringa, prawie nigdy nie musi pamiętać o sprawdzeniu jego poprawności:
public class DriverService {
public Driver createDriver(String license, String lastName,
String firstName, Driver.Type type,
Driver.Status status, String photo) {
// ...
if (status.equals(Driver.Status.ACTIVE)) {
driver.setDriverLicense(withLicense(newLicense));
} else {
driver.setDriverLicense(withoutValidation(newLicense));
}
// ...
}
public void changeLicenseNumber(String newLicense, Long driverId) {
// ...
driver.setDriverLicense(withLicense(newLicense));
// ...
}
public void changeDriverStatus(Long driverId, Driver.Status status) {
// ...
if (status.equals(Driver.Status.ACTIVE)) {
try {
driver.setDriverLicense(withLicense(driver.getDriverLicense().asString()));
} catch (IllegalArgumentException e) {
throw new IllegalStateException(e);
}
}
// ...
}
}
Widać poprawę, a sama praca nie należy do skomplikowanych.
📈 Czy nie kusi Cię ten zwrot z inwestycji?
Czy nie mamy duplikacji w testach?
Spójrz na ten test napisany na początku prac, który sprawdza, czy można utworzyć kierowcę z niepoprawnym numerem prawa jazdy:
class ValidateDriverLicenseIntegrationTest {
//..
@Test
void cannotCreateActiveDriverWithInvalidLicense() {
//expect
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> createActiveDriverWithLicense("invalid"));
}
//..
}
A teraz spójrz na ten test:
class DriverLicenseTest {
@Test
void cannotCreateInvalidLicense() {
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> DriverLicense.withLicense("invalid"));
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> DriverLicense.withLicense(""));
}
//..
}
Czy widzisz tu jakiś problem? Może duplikację? Jakbyś podeszła lub podszedł do tego typu zagadnienia? Przecież duplikacja jest zła. Zastanów się, czy któryś rodzaj testów jest nadmiarowy. Może usunąć testy integracyjne? A może jednostkowe? A może, przychodzi Ci do głowy jakieś inne rozwiązanie, które można by było zastosować w Twoim projekcie?
Zbiór powodów, dla których test jednostkowy przestanie działać, zawiera się w zbiorze powodów, dla których test integracyjny będzie czerwony. Jeśli zmienimy sposób walidacji albo popełnimy w instrukcjach walidujących błąd, to oba testy przestaną działać dokładnie z tego samego powodu.
Pracę zaczęliśmy od testowania integracyjnego, bo walidacja była rozsiana, nie było jednego miejsca, o stabilnym interfejsie, którego wywołanie sprawdzałoby instrukcje poprawności numeru prawa jazdy. Takim miejscem jest nowopowstałe DriverLicense, które ma swój test. Jest to test, który jest szybszy, przez co ma szybszą pętlę informacji zwrotnej. Jest to test, który łatwiej napisać i utrzymać — nie trzeba zestawiać wielu komponentów, w tym testowej bazy danych.
Test jednostkowy a integracyjny:
- test jednostkowy przestanie działać z tego samego powodu, dla którego załamał się test integracyjny
- test jednostkowy jest szybszy
- test jednostkowy jest tańszy w utrzymaniu
W systemach klasy legacy to częsta sytuacja — zabezpieczamy nasze pole do zmian testem wysokiego poziomu, w środku destylujemy nowe obiekty, które przejmują jakąś odpowiedzialność. One dostają swoje testy jednostkowe. Część z tych testów będzie nadmiarowa, więc należy się zastanowić co z nimi zrobić.
Można na przykład usunąć testy integracyjne, ponieważ wypełniły swoją rolę. Wtedy często pojawia się emocja przywiązania, bo kosztowały Cię sporo pracy i nie chcesz ich usuwać. Przywiązanie się nie jest jednak najlepszą strategią, kod to narzędzie, które ma w danym momencie swoją funkcję.
Z drugiej strony, raptowne usunięcie wszystkiego to drugie ekstremum. Postaraj się więc inżynieryjnie i pragmatycznie ocenić jakie podejście będzie najlepsze.
Wracając do omawianego testu integracyjnego:
class ValidateDriverLicenseIntegrationTest {
//..
@Test
void canCreateActiveDriverWithValidLicense() {
//when
Driver driver = createActiveDriverWithLicense("FARME100165AB5EW");
//then
DriverDTO loaded = load(driver);
assertEquals("FARME100165AB5EW", loaded.getDriverLicense());
assertEquals(ACTIVE, loaded.getStatus());
}
//..
}
Sprawdźmy jakie unikalne bezpieczeństwo nam daje. "Unikalne" czyli takie, którego nie daje nam test jednostkowy — to rozwiąże nam problem duplikacji.
Test integracyjny przechodzi przez 2 obserwowalne zachowania:
- utwórz kierowcę,
- załaduj jego stan z bazy danych.
Te metody przechodzą przez proces utworzenia encji, ustawienia jej stanu, persystencji i pobierania z bazy:
public class DriverService {
//..
public Driver createDriver(String license, String lastName,
String firstName, Driver.Type type,
Driver.Status status, String photo) {
Driver driver = new Driver();
if (status.equals(Driver.Status.ACTIVE)) {
driver.setDriverLicense(withLicense(license));
} else {
driver.setDriverLicense(DriverLicense.withoutValidation(license));
}
//..
if (photo != null && !photo.isEmpty()) {
if (Base64.isBase64(photo)) {
driver.setPhoto(photo);
} else {
throw new IllegalArgumentException("Illegal photo in base64");
}
}
return driverRepository.save(driver);
}
//..
public DriverDTO loadDriver(Long driverId) {
Driver driver = driverRepository.getOne(driverId);
if (driver == null) {
throw new IllegalArgumentException("Driver does not exists, id = " + driverId);
}
return new DriverDTO(driver);
}
}
Metody te sprawdzają dużo więc więcej niż tylko ten kawałek:
if (status.equals(Driver.Status.ACTIVE)) {
driver.setDriverLicense(withLicense(license));
} else {
driver.setDriverLicense(DriverLicense.withoutValidation(license));
}
Tym sposobem test integracyjny daje nam większe bezpieczeństwo, np. pozwoli nam sprawdzić, że nasz nowy obiekt DriverLicense potrafi być zapisany do bazy danych.
Inaczej sprawa ma się jednak z testem integracyjnym niepozwalającym na użycie niepoprawnego numeru:
class ValidateDriverLicenseIntegrationTest {
//..
@Test
void cannotCreateActiveDriverWithInvalidLicense() {
//expect
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> createActiveDriverWithLicense("invalid"));
}
//..
}
Nie zapisuje on nic w bazie, nie sprawdza stanu kierowcy (wyłączając drobne szczegóły), nie daje nam większego bezpieczeństwa niż korespondujący test jednostkowy.
Rozwiązaniem mogłoby być zostawienie tzw. happy path w teście integracyjnym — test sprawdzi, czy komponent bazy danych integruje się z komponentem DriverLicense, ale nie musi robić tego w koniunkcji z każdą możliwą postacią numeru prawa jazdy. Podobnie ze zmianą numeru prawa jazdy — happy path na to zachowanie wydaje się wystarczający.
Podsumowując cały proces, wygląda on tak:
- zabezpieczenie testami wyższego poziomu,
- destylacja mniejszych komponentów z testami jednostkowymi,
- ocena poziomu duplikacji. np. pozostawienie "happy path" w testach wyższego poziomu.
To jest oczywiście, nasze, subiektywne spojrzenie na ten problem. Sprawdzało się w projektach, w których mieliśmy okazję uczestniczyć.
Jak to zrobić w moim projekcie?
Punktem wyjścia jest tu oczywiście zauważenie i nazwanie problemu wynikającego z powielonej logiki walidacyjnej. Oznaczało to modyfikowanie kilku podobnie wyglądających do siebie miejsc, przy każdej zmianie wymagań.
Omawiane "zauważenie" powtórzeń można zlecić automatom. Narzędzia do statycznej analizy kodu źródłowego pozwolą wychwycić tego rodzaju powiązania, np. gdy zmiany są wprowadzane:
- w odrębnych częściach aplikacji, ale w ramach tej samej zmiany w repozytorium,
- przez tego samego autora, ale występują na przestrzeni zdefiniowanego okresu i gdy nie widać bezpośredniego połączenia pomiędzy zmienianymi miejscami,
- przez różnych autorów, ale są opisane w ten sam sposób,
- w całkowicie osobnych repozytoriach, ale są opisane w ten sam sposób lub występują w podobnych momentach, co może być szczególnie przydatne w przypadku np. architektury mikroserwisowej.
Opcji jest tu całkiem sporo i nie trzeba do końca polegać na naszej dogłębnej znajomości każdego fragmentu systemu i historii repozytorium, ale działałaby ona tylko na naszą korzyść.
Nic nie zastąpi tu jednak twojej oceny sytuacji w konkretnym projekcie. Raport automatycznego przeglądu kodu powinien tu być tylko sygnałem do zajęcia się tematem. Być może po zapoznaniu się z jego wynikami dojdziesz do wniosku, że wprowadzanie obiektu enkapsulującego powtarzającą się logikę ma głębokie uzasadnienie i pozytywnie wpłynie na projekt. Pamiętaj też o wspominanej idei stabilnego i zmiennego kodu.
Większą pewność co do kierunku rozwoju systemu uzyskasz także, znajdując odpowiedzi na poniższe pytania:
- Jak dużym problemem dla projektu i zespołu jest powielenie logiki walidacyjnej?
- W ilu miejscach logika walidacyjna została powtórzona?
- Jaka jest historia zmian tej logiki?
- Czy w niedalekiej przyszłości planowane są jej zmiany?
- Czy potencjalna zmiana jest jednorazowa?
- Czy korzystanie z silnego typowania i nowego typu przyniesie zysk?
Jak widzisz, ciągle należy uzasadnić wysiłek powiązany z potrzebną pracą.
Jeśli znasz wszystkie warte uwagi odpowiedzi i chcesz przeprowadzić cały proces, twoje dalsze działania mogą być wyznaczone Generalną Ścieżką Refaktoryzacji:
- Zapewnij bezpieczeństwo zmiany poprzez wprowadzenie na odpowiednim poziomie testu opartego o odkryte obserwowalne zachowania.
- Wprowadź nowy koncept do systemu i umieść w nim problematyczną logikę walidacji.
- Podłącz nowy obiekt i upewnij się, że obserwowalne zachowania pozostały niezmienione.
- Zaobserwuj osiągnięty efekt i podejmij decyzję odnośnie do dalszych działań.
Od zbudowania bezpieczeństwa, na przykład testem na odpowiednim poziomie, wprowadzeniem odpowiedniego obiektu i przeniesieniem do niego problematycznej logiki, wykorzystania nowego obiektu, aż po obserwację efektu i ewentualną korektę planu.
Może jednak wyciągniesz zupełnie odmienne wnioski i ze względu na odkrycie, że ta logika jest zależna od stanu systemu, zdecydujesz się na użycie zupełnie innych technik.
Użycie poszczególnych narzędzi, technik i wzorców dopasowujemy zawsze do konkretnej sytuacji projektu i jego problemów.
Dodatkowe komentarze
Na koniec zachęcamy Cię jeszcze do obejrzenia filmu: "Dodatkowe komentarze", w którym odpowiadamy na następujące pytania:
- Dlaczego klasa DriverLicense posiada metodę withoutValidation?
- Dlaczego taki kod?
- Dlaczego zmieniany jest typ wyjątku?
Podsumowanie
Warto zapamiętać:
powtórka kodu nie zawsze jest problemem
gorsza jest powtórzona logika
w szczególności, gdy jest zmienna i powtórzone miejsca muszą być zmieniane jednocześnie
w legacy (przed refaktoryzacją) często możliwe do implementacji są tylko testy wyższego poziomu
napisanie testu w legacy daje dużo więcej niż siatkę bezpieczeństwa
oceniając poziom duplikacji testów warto zadać sobie pytania: "co sprawdza ten test?" i "jaka jest jego unikalna wartość?"
Materiały: