DevCezz

Programistyczny blog dla Ciebie

Czy Ty też tak robisz z polem status?
Poradniki

Czy Ty też tak robisz z polem status?

Chciałbym zaprezentować Wam moje przemyślenia na temat, znanego na pewno przez wszystkich programistów, pola status. W każdym projekcie w jakim pracowałem było obecne właśnie pole o takiej nazwie. Implementowane było najczęściej jako typ wyliczeniowy, a rzadziej (na szczęście) jako integer.

Studium przypadku

Najłatwiej wytłumaczyć dane zagadnienie na jakimś konkretnym przypadku. Zacznijmy więc od zdefiniowania podstawowych wymagań. Załóżmy, że implementujemy system do obiegu dokumentów w postaci ofert. Chcielibyśmy, aby zmieniały one swój status w zależności od akcji jaką wykonamy. Do dyspozycji mamy, więc takie o to opcje.

  • Draft
    • wyślij do akceptacji = Oczekujący
    • usuń = Usunięty
  • Oczekujący
    • akceptuj = Zaakceptowany
    • usuń = Usunięty
  • Zaakceptowany
    • wyślij do poprawki = Draft
  • Usunięty
    • wyślij do poprawki = Draft

Prościej oczywiście można zaprezentować te wymagania przy pomocy grafiki. Prezentuje się ona w następujący sposób.

Skoro mamy już wszelkie niezbędne informacje to teraz czas zakasać rękawy i wziąć się za implementację tego rozwiązania.

Pierwsza iteracja

Pierwszą myślą jaka może pojawić się w naszej głowie jest zastosowanie wcześniej wspomnianego typu wyliczeniowego o jakże błyskotliwej nazwie Status. Będzie ono posiadało 4 możliwości odpowiadające naszym wymaganiom. Umieścimy je jako pole w klasie Offer i w zależności od akcji będzie ono ulegało zmianie odpowiadając danemu stanowi. No to lecimy!

enum Status {
    Pending, Accepted, Draft, Deleted
}
@AllArgsConstructor(access = AccessLevel.PRIVATE)
@Getter(AccessLevel.PACKAGE)
class Offer {

    private final Number number;
    private Status status;

    static Offer create(Number number) {
        return new Offer(number, Status.Draft);
    }

    void sendToAcceptance() {
        if (status != Status.Draft) {
            throw new IllegalArgumentException();
        }
        this.status = Status.Pending;
    }

    void accept() {
        if (status != Status.Pending) {
            throw new IllegalArgumentException();
        }
        this.status = Status.Accepted;
    }

    void delete() {
        if (status == Status.Accepted || status == Status.Deleted) {
            throw new IllegalArgumentException();
        }
        this.status = Status.Deleted;
    }

    void sendToCorrection() {
        if (status != Status.Accepted && status != Status.Deleted) {
            throw new IllegalArgumentException();
        }
        this.status = Status.Draft;
    }
}

Jak widzisz w każdej z metod musimy dokonać walidacji czy, aby oferta znajduje się w odpowiednim stanie, żeby móc na niej wykonać określoną akcje. Warto zwrócić uwagę, że do testów musieliśmy dodać getter na polu status. W ten sposób będziemy mogli zweryfikować czy oferta faktycznie znajduje się w odpowiednim stanie.

def "should send draft offer to acceptance"() {
    given:
        Offer offer = Offer.create(anyNumber())

    when:
        offer.sendToAcceptance()

    then:
        offer.status == Status.Pending
}

def "should fail when accept draft offer"() {
    given:
        Offer offer = Offer.create(anyNumber())

    when:
        offer.accept()

    then:
        thrown(IllegalArgumentException.class)
}

Obydwa testy jak najbardziej przechodzą. Wszystko działa, więc zadanie wydaje się być wykonane. Jednak nie do końca. Co w przypadku, gdyby doszedł nam nowy status o nazwie np. 'Wymaga załącznika’? Musielibyśmy go zaimplementować i w dodatku wypadałoby przejrzeć wcześniej zaimplementowane metody pod kątem walidacji. Być może oferta będąc w statusie ’Wymaga załącznika’ nie może zostać usunięta i warto byłoby to uwzględnić. Czy istnieje zatem lepszy sposób na napisanie tego kawałka kodu? Sprawdźmy to.

Druga iteracja

Spróbujmy dodać interfejs, który będzie miał w sobie wcześniej przedstawione akcje do zaimplementowania. Dodatkowo stworzymy klasy reprezentujące każdy stan przedstawiony w typie wyliczeniowym. Każdy z nich będzie implementować właśnie nasz interfejs. Tutaj może pojawić się pytanie. Skoro każdy stan ma swoją osobną klasę to czy niezbędne jest teraz pole status? Odpowiedź brzmi, nie. Wynikiem działania danej metody będzie zwrócenie interfejsu, za którym będzie kryła się implementacja jednego ze stanów. Już pokazuję jak to wygląda w kodzie dla statusu 'Draft’ oraz 'Oczekujący’.

interface Offer {

    Offer sendToAcceptance();

    Offer accept();

    Offer delete();

    Offer sendToCorrection();
}

@AllArgsConstructor(access = AccessLevel.PACKAGE)
class DraftOffer implements Offer {

    private final Number number;

    @Override
    public Offer sendToAcceptance() {
        return new PendingOffer(number);
    }

    @Override
    public Offer accept() {
        throw new IllegalArgumentException();
    }

    @Override
    public Offer delete() {
        return new DeletedOffer(number);
    }

    @Override
    public Offer sendToCorrection() {
        throw new IllegalArgumentException();
    }
}

@AllArgsConstructor(access = AccessLevel.PACKAGE)
class PendingOffer implements Offer {

    private final Number number;

    @Override
    public Offer sendToAcceptance() {
        throw new IllegalArgumentException();
    }

    @Override
    public Offer accept() {
        return new AcceptedOffer(number);
    }

    @Override
    public Offer delete() {
        return new DeletedOffer(number);
    }

    @Override
    public Offer sendToCorrection() {
        throw new IllegalArgumentException();
    }
}

//... AcceptedOffer i DeletedOffer

Wygląda to już znacznie lepiej. Nie mamy żadnego, nic nie mówiącego pola status. Widnieją za to 4 klasy reprezentujące osobne stany w jakich może znaleźć się dana oferta. Sprawdźmy zatem jak wyglądają wcześniej przedstawione testy dla tego rozwiązania.

def "should send draft offer to acceptance"() {
    given:
        Offer draftOffer = new DraftOffer(anyNumber())

    when:
        def pendingOffer = draftOffer.sendToAcceptance()

    then:
        pendingOffer instanceof PendingOffer
}

def "should fail when accept draft offer"() {
    given:
        Offer draftOffer = new DraftOffer(anyNumber())

    when:
        draftOffer.accept()

    then:
        thrown(IllegalArgumentException.class)
}

Na pierwszy rzut oka praktycznie nie widać różnicy. Jednak zmianie uległa asercja w pierwszy teście. Zamiast weryfikować przez getter status naszej oferty sprawdzamy czy dany obiekt to faktycznie instancja klasy PendingOffer. Zewnętrzny użytkownik nie wie nic o wewnętrznej strukturze rozwiązania naszego problemu.

A co w przypadku nowego status? Dodalibyśmy po prostu nową klasę implementującą interfejs Offer i tyle. Wychodzi więc na to, że wszystko jest super i możemy skończyć w tym momencie. No dalej nie do końca. Co w przypadku, gdy dojdzie nam nowa akcja? Dopiszemy ją do interfejsu Offer i każda implementująca go klasa będzie musiała ulec modyfikacji. Łamiemy, więc w jawny sposób zasadę Open Closed-Principle z mnemonika SOLID. Dodatkowo Pani Barbara Liskov też nie byłaby z nas dumna. Jej zasada również została naruszona. Musimy zatem pomyśleć nad innym rozwiązaniem.

Trzecia iteracja

Gdyby tak wyciągnąć część wspólną dla tych wszystkich klas do jednego interfejsu, a resztę zostawić bez zmian? Oczywiście usuniemy wtedy zbędne metody tam gdzie nie są one potrzebne. Sprawdźmy, więc jakby mogłoby wyglądać to rozwiązanie.

interface Offer {
    Number getNumber();
}

@AllArgsConstructor(access = AccessLevel.PACKAGE)
@Getter(AccessLevel.PACKAGE)
class DraftOffer implements Offer {

    private final Number number;

    Offer sendToAcceptance() {
        return new PendingOffer(number);
    }

    Offer delete() {
        return new DeletedOffer(number);
    }
}

@AllArgsConstructor(access = AccessLevel.PACKAGE)
@Getter(AccessLevel.PACKAGE)
class PendingOffer implements Offer {

    private final Number number;

    Offer accept() {
        return new AcceptedOffer(number);
    }

    Offer delete() {
        return new DeletedOffer(number);
    }
}

//... AcceptedOffer i DeletedOffer

Kod znowu wygląda podobnie do tego przedstawionego wcześniej. Jednak jest go o wiele mniej. Nie mamy metod przestawiających akcje w klasach, gdzie nie są one konieczne. To teraz pora na spojrzenie na testy.

def "should send draft offer to acceptance"() {
    given:
        DraftOffer draftOffer = new DraftOffer(anyNumber())

    when:
        def pendingOffer = draftOffer.sendToAcceptance()

    then:
        pendingOffer instanceof PendingOffer
}

//def "should fail when accept draft offer"() {
//    given:
//        DraftOffer draftOffer = new DraftOffer(anyNumber())
//
//    when:
//        draftOffer.accept()
//
//    then:
//        thrown(IllegalArgumentException.class)
//}

Okazuje się, że test weryfikujący możliwość akceptacji oferty w stanie 'Draft’ się nie kompiluje. Po prostu metoda ją reprezentująca nie istnieje. Czyli dostaliśmy dodatkowe zabezpieczenie całkiem za darmo. Nie musimy się martwić żadnymi weryfikacjami czy oferta jest w odpowiednim stanie. Kompilator zrobi to za nas. Gdybyśmy teraz chcieli dodać nową akcję to musimy tylko wejść do klasy reprezentującej konkretny stan i ją tam dodać. Nic więcej. Wszystkie inne klasy zostaną nienaruszone.

Użycie vavr

Zdaję sobie jednak sprawę, że gdzieś musi zostać dokonane rzutowanie naszego interfejsu na konkretną klasę, aby móc wykonać odpowiednią akcję. Faktycznie niektórym osobom może to przeszkadzać. Dla mnie osobiście nie jest to nic strasznego. Zwłaszcza dlatego, że można wykorzystać bibliotekę vavr do uproszczenia nam życia.

Zamiast pisać instrukcje warunkowe if albo switch możemy wykorzystać klasę Match. Dzięki przyjaznemu API możemy przekazać do niej nasz obiekt, a potem w jej warunkach sprawdzać czy jest to instancja danej klasy. Brzmi to bardzo podobnie jak switch, ale nie musimy tutaj robić żadnego rzutowania czy wykorzystywać innych możliwości językowych. Użycie tej konstrukcji prezentuje się następująco.

Offer sendToAcceptance(Offer offer) {
    return Match(offer).of(
        Case($(instanceOf(DraftOffer.class)), DraftOffer::sendToAcceptance),
        Case($(), () -> offer)
    );
}

Jak widzisz API jest naprawdę przyjemne w czytaniu. Wszystkie techniczne operacje dzieją się za naszymi plecami. Dodatkowo taką metodę można wykorzystywać w łatwy sposób jako lambdę w jednej z części strumienia Stream.

Materiał dodatkowy

Powyższe rozwiązanie ma jeden minus. Jeśli dojdzie nam akcja sendToAcceptance dla innego stanu to będziemy musieli zmienić nasz Match. Można, więc pójść o krok dalej. Zastosujmy w tym miejscu zasadę Segregacji Interfejsów. W przypadku, gdy oferta jest w stanie 'Draft’ albo 'Oczekujący’ to możemy na niej wykonać akcję usuwania. Może lepiej w takim razie byłoby dodać dla tej akcji dedykowany interfejs?

//... interface Offer

interface DeleteOfferAction {
    DeletedOffer delete();
}

@AllArgsConstructor(access = AccessLevel.PACKAGE)
@Getter(AccessLevel.PACKAGE)
class DraftOffer implements Offer, DeleteOfferAction {

    private final Number number;

    Offer sendToAcceptance() {
        return new PendingOffer(number);
    }

    @Override
    public DeletedOffer delete() {
        return new DeletedOffer(number);
    }
}

@AllArgsConstructor(access = AccessLevel.PACKAGE)
@Getter(AccessLevel.PACKAGE)
class PendingOffer implements Offer, DeleteOfferAction {

    private final Number number;

    Offer accept() {
        return new AcceptedOffer(number);
    }

    @Override
    public DeletedOffer delete() {
        return new DeletedOffer(number);
    }
}

//... AcceptedOffer i DeletedOffer

Teraz instrukcja Match z vavr wyglądałaby znacznie lepiej. Nie ulegałaby żadnym modyfikacjom. Jeśli doszedłby kolejny stan to musiałby on tylko zaimplementować interfejs DeleteOfferAction i natychmiast zostałby obsłużony przez poniższy kod.

Offer delete(Offer offer) {
    return Match(offer).of(
        Case($(instanceOf(DeleteOfferAction.class)), DeleteOfferAction::delete),
        Case($(), () -> offer)
    );
}

Podsumowanie

Mam nadzieję, że tym artykułem przybliżyłem Wam inne możliwości zobrazowania sobie statusu w jakim znajduje się dany obiekt. Chcę zaznaczyć, że jest to tylko mój punkt widzenia na ten problem. Jeśli masz inny pomysł lub widzisz jakieś luki w moim rozumowaniu to daj znać.

Na koniec dodam, że oczywiście baza danych może mieć swój enum reprezentujący status na podstawie, którego będzie zwracała odpowiedni obiekt domenowy. Nie ma w tym nic złego. Moją intencją było po prostu przedstawienie innego postrzegania kodu domenowego. Z powyższym kodem trudniej w nim o pomyłkę czy jakieś niedopatrzenie.

Podziel się tym z innymi!

2 KOMENTARZE

  1. > Okazuje się, że test weryfikujący możliwość akceptacji oferty w stanie 'Draft’ się nie kompiluje.

    sendToAcceptance() też się nie kompiluje bo Offer ma tylko metodę getNumber()

    To jest powiedzmy literówka.

    Ogólnie pomysł jest ciekawy. Ale nie rozumiem o co chodzi w
    „Zdaję sobie jednak sprawę, że gdzieś musi zostać dokonane rzutowanie naszego interfejsu na konkretną klasę, aby móc wykonać odpowiednią akcję”

    Wydawało mi się że celem trzeciej iteracji żeby mieć kod który zawsze operuje na typach DraftOffer , PendingOffer. I wtedy nie ma możliwości aby wywołać niedozwoloną metodę: Bo DraftOffer udostępnia tylko sendToAcceptance(), PendingOffer tylko accept() itd.
    Czyli że celem trzeciej iteracji jest to żeby kompilator weryfikował poprawność stanu. A używają rzutowania wszystkie zalety tracimy i mamy kod mniej czytelny niż po pierwszej iteracji

    • Dzięki michaldo za poprawkę, faktycznie tam powinno być DraftOffer a nie Offer. 🙂

      Co do zdania – „Zdaję sobie jednak sprawę, że gdzieś musi zostać dokonane rzutowanie naszego interfejsu na konkretną klasę, aby móc wykonać odpowiednią akcję”. Chodziło o to, że teraz trzeba robić weryfikację z instanceof co nie jest za przyjemne i w tym pomaga nam vavr. Jednak faktycznie zatraciłem tutaj zaletę tego podejścia. Pytając repozytorium o obiekt faktycznie możemy zwrócić już daną implementacje w Optionalu, jeśli jej nie znajdziemy to dajemy empty. Wtedy to kompilator będzie nam już na 100% pilnował, abyśmy wywołali prawidłową metodę. Więc masz rację i dzięki za uwagę. Jesteś jak zawsze czujny! 🙂

      Pozdrawiam serdecznie!

ZOSTAW ODPOWIEDŹ

Twój adres e-mail nie zostanie opublikowany.