Value objecty - walidacja

Dzień dobry
Value objecty to świetny sposób na modelowanie domeny, np. klasa Pesel jest o wiele bardziej prezycyjna niż String i łatwiej będzie zrozumiec o co chodzi. Jednak pojawia się pewien "problem".
Jesli mamy taka klase:

public final class Pesel {
 private final String value;
 
private Pesel(String value) {
  this.value = value;
 }

public static Pesel of(String pesel) {
  return new Pesel(pesel);
 }
}

Czy to nie powinniśmy w takiej metodze wytwórczej sprawdzać czy wartośc jest prawidłowa? Jesli tak, to co zrobić jeśli jest to wartośc nieprawidłowa? Wyrzucac runtimowy expetion?

To zależy co rozumiesz poprzez prawidłową wartość. Jeśli to czy taki pesel faktycznie istnieje i aby to sprawdzić musisz komunikować się z jakimś serwisem to nie. Natomiast jeśli chodzi o walidację struktury samej wartości (np. czy ma określoną długość) to tak, powinieneś to sprawdzić i rzucić wyjątek jeśli wartość się nie zgadza. Value Objects podobnie jak "zwykłe" klasy powinny strzec swoich zasad.

Natomiast jeśli chodzi o walidację struktury samej wartości (np. czy ma określoną długość) to tak, powinieneś to sprawdzić i rzucić wyjątek jeśli wartość się nie zgadza.

O to mi dokładnie chodziło, są reguły walidacji, np. w przypadku PESEL jest to suma kontrolna. Widziałem w kodzie takie obiekty których nie było to sprawdzane i stwierdziłem że coś nie tak

A moze w takim razie:

public static Optional<Pesel> parsePesel(String pesel){

?

Jeśli te dane pochodzą gdzieś z frontu to to, że są nieprawidłowe wcale nie jest jakimiś wyjątkiem ;) Tak jak Shalom napisał albo Optional z SDK albo jak chcesz iść dalej w to to Either, gdzie możesz też zwrócić powód dla którego nie udało się coś wykonać

Ja bym w konstruktorze wytwórczej dał co najwyżej null/empty checka z runtime exception. Jakieś walidacje czy prawidłowy pesel etc do osobnych metod/klas

No to ja wiem że jesli to wartośc z frontu to lepiej nie rzucać zawsze wyjątkiem, zawsze można zrobić 2 metody fabryczne ;)

baant napisał(a):

Ja bym w konstruktorze dał co najwyżej null/empty checka z runtime exception. Jakieś walidacje czy prawidłowy pesel etc do osobnych metod/klas

Skoro jest metoda wytwórcza to czemu pchać walidację do konstruktora? Jedną z większych zalet metod wytwórczych jest to, że dzięki nim można uniknąć efektów ubocznych w konstruktorze.

Według mnie, w takim przypadku konstruktor klasy powinien być prywatny (tak jak jest w Twoim przykładzie) i jedna metoda tworząca dany obiekt z pełną walidacją. Jako wynik albo Option, albo Either w zależności czy zależy Ci na przyczynie błędu czy nie. W ten sposób masz pewność, że w każdej metodzie przyjmującej typ Pesel masz poprawnie skonstruowany obiekt.

Typowym rozwiązaniem (zaimplementowanym np. w Paths.get) jest metoda wytwórcza rzucająca wyjątek, np. IllegalArgumentException. Ja bym poszedł w tą stronę. Walidacja dla użytkownika może być przecież w całkiem innym miejscu. Dołożyłbym też pewnie statyczna metoda 'validate`.

@tdudzik: to jest imo najgorsze rozwiązanie, bo robi coś czego się nie spodziewasz. Patrząc na sygnaturę 

public static Path get(String first, String... more)

Nie powinieneś się spodziewać (no dobra, większość się spodziewa, ale imo nie powinno sie) wyjątku w momencie raczej dość standardowym czyli podaniu złej ścieżki. Z resztą w jednym wątku podobnym podałem przykład

int parseInt(String s); //w sumie nie wiesz co się stanie bez czytania dokumentacji jak podasz "a"
Optional<Integer> parseInt(String s); //tu brak info o błędzie, ale już wiesz, że jakiś błąd może wystąpić
Either<ParseError, Integer> parseInt(String s); //info o ewentualnym błędzie. Wada: mało popularne podejście

Dokładnie tak samo można zrobić w przypadku tworzenia obiektów. Jedyne gdzie wyjątek faktycznie może zostać użyty, to przy "ponownym" sprawdzeniu czegoś w prywatnym konstruktorze, ale oznacza to, że musiałbyś dwa razy sprawdzać to samo i za pierwszym razem źle.

Co innego jeśli masz założenie, że dane wejsciowe będą poprawne (np wczytanie pliku konfiguracyjnego) a ich tam nie będzie. Wtedy to jest sytuacja wyjątkowa

@tdudzik: po 1 Path było zanim było Optional, po 2 to że tak jest w API Javy to nie znaczy ze jest dobre. Calendar tez był a raczej dalej jest w API Javy...

Skoro mowa o Value objectach to mowa o DDD. Więc nie powinno się dać stworzyć niepoprawnego Peselu, np "1234" albo "Ala ma kota". Także owszem, w konstruktorze powinieneś to sprawdzać. Albo w fabryce, jeśli zasady dla stworzenia poprawnego obiektu są dość skomplikowane i chcesz je gdzieś wydzielić.

Także oprócz sprawdzenia klasyka, typu null, pusty string potrzebujesz jeszcze poprawności logicznej, może nawet sprawdzenia sumy kontrolnej w wypadku numeru pesel.

Skoro mowa o Value objectach to mowa o DDD.

Wybacz szczerość, ale skąd ta absurdalna teza?

Bo:

1. Pojawia się dużo tematów na temat DDD.
2. Napisałeś o jednym z jego klocków.
3. Napisałeś o modelowaniu domeny - do czego się zwykle używa DDD.

Więc uznałem za rozsądne takie założenie. Swoją drogę, nawet jeśli pytasz w innym kontekście, to moja odp będzie taka sama. Wolę nie móc stworzyć nie poprawnego obiektu, niż móc to spartaczyć, bo ktoś zapomni wywołać walidatora wcześniej.

Prawde powiedziawszy pytałem w kontekście archiektury cebulowej czyli hexagonalnej

Późno jest, więc pomysły dziwne mi przychodzą do głowy, więc może zamiast oklepanych metod może taka, która tworzy "kulawe" PESELE jako obiekty osobnej klasy:

class Pesel{
    public static final class Valid extends Pesel{
        private Valid(String peselStr) {
            super(peselStr);
        }

        public String birthYear(){
            return pesel.substring(0,2);
        }

        public String birthMonth(){
            return pesel.substring(2,4);
        }

        public String birthDay(){
            return pesel.substring(4, 6);
        }

        @Override
        public String toString() {
            return "Valid{" +
                    "pesel='" + pesel + '\'' +
                    '}';
        }
    }

    public static final class Wrong extends Pesel{
        private Wrong(String peselStr) {
            super(peselStr);
        }

        public String errors(){
            return "Wrong length";
        }

        @Override
        public String toString() {
            return "Wrong{" +
                    "pesel='" + pesel + '\'' +
                    '}';
        }
    }

    protected String pesel;

    private Pesel(String peselStr){
        pesel = peselStr;
    }
    public static Pesel of(String peselStr){
        return valid(peselStr) ? new Valid(peselStr) : new Wrong(peselStr);
    }

    private static boolean valid(String peselStr){
        return peselStr.length() == 11;
    }

    public boolean isValid(){
        return this instanceof Valid;
    }

    public boolean isWrong(){
        return this instanceof Wrong;
    }

    public String getPesel(){
        return pesel;
    }
}

public class ValueObjectDemo {
    public static void main(String[] args) {

        List<Pesel> list = Arrays.asList(Pesel.of("69110828130"), Pesel.of("4573945387"), Pesel.of("7891827981739"));
        System.out.println("Valid PESEL's");
        list.stream().filter(Pesel::isValid).map(a -> (Pesel.Valid) a).forEach(p -> System.out.println(p + ", year: " + p.birthYear() + ", month: " + p.birthMonth()));
        System.out.println("Wrong PESEL's");
        list.stream().filter(Pesel::isWrong).map(a-> (Pesel.Wrong) a).forEach(p -> System.out.println(p.errors()));
    }
}

@cs: Takie rozwiązanie na pierwszy rzut oka ma co najmniej dwa problemy:
Pierwszym z nich jest to, że ktoś przyjdzie, zrobie refactoring, usunie bądź zmieni pozycję filter(Pesel::isWrong) i kod może rzucać błędami.
Drugim jest fakt, że z takim podejściem może być problematyczne dodawanie kolejnych błędów. Każdy kolejny błąd to kolejna metoda sprawdzająca (isChecksumValid, isOnlyDigits i tak dalej), kolejne rzutowania i kolejne pola do błędów.

Rozwiązaniem mogłoby być zamodelowanie błędów jako ADT, a typ zwracany przez metodę tworzącą mógłby mieć sygnaturę Either<PeselValidationError, Pesel> bądź Validation<List<PeselValidationError>>, Pesel>

Dokładnie, są tylko 3 rozwiązania:
1)rzucanie wyjątków
2)Optional (pusty dla nieprawidłowej wartości)
3)Either
inne sa po prostu złe

@scibi92: Przy czym dodałbym:

wyjątek - autor API zakłada, że nikt złych danych nie powinien przekazać. Jak przekaże to prawdopodobnie inna logika zawiodła (walidacja / GUI). Rzuca wyjątek by podnieść alarm, zaczerwienić monitoring - zmusić kogoś do działania. W pewnych bardzo rzadkich kontekstach może też rzucić checked exception by dać wołającemu szansę na naprawę stanu i ponowienie.

optional / either - autor API zakłada, że złe dane będą przekazywane i nie należy panikować. Rzucanie wyjątków nie ma tutaj sensu. Wyjątki są wolne, nie optymalizowane przez JIT, tworzą stacktracey na heapie.

scibi92 napisał(a):

Dokładnie, są tylko 3 rozwiązania:
1)rzucanie wyjątków
2)Optional (pusty dla nieprawidłowej wartości)
3)Either
inne sa po prostu złe

@scibi92: Nie byłbym taki kategoryczny, u nas we wiosce mówią: jakbyś nie obrócił to i tak d..a zawsze zostanie z tyły. Każde z rozwiązań ma swoje "ale":

1. rzucanie wyjątków - cóż to za wyjątkowa sytuacja, że user się powyli, wyjątki wprowadzono dawno temu jako jednolitą formę reakcji na tak różne błędy od krytycznych po drobnostki
2. Optional - ok ale jak chcemy dowiedzieć się dlaczego nie udało się obiektu utworzyć to brak informacji o przyczynie
3. Either - ma to do siebie, że w razie niepowodzenia można zwrócić wszystko, co dusza zapragnie, chcę utworzyć pesel a dostaje opakowanie w którym może być pesel albo nie.

Jak dla mnie rozwiązanie zależy od sytuacji, czyli w którą stronę chcesz się obrócić i co tam chcesz zobaczyć. Szukanie rozwiązania, że nam z tyłu d..a zniknie jest utopią.

@danek
Problem z Either jest taki, że metoda wytwórcza klasy A powinna tworzyć obiekt tej klasy, a tu wyskakuje jakiś Either, który może zawierać żądany obiekt albo coś innego. Czy to jest intuicyjne?

To o co mi chodziło w tym rozwiązaniu to to, żeby metoda wytwórcza zwracała zawsze obiekt danego typu, a przez dziedziczenie segreguje, który obiekt jest poprawny a który kulawy. Poprawny ma metody przetwarzające dane, o których wiemy, że są na 100% poprawne, a błędny może mieć metody informujące, dlaczego nie jest taki, czego mu brakuje i zawiera surowe dane.

1)Jeśli zakładamy że VO tworzymy na podstawie jakiś DTOsów z zewnątrz i one zawierają błędne dane po walidacji to jest to sytuacja wyjątkowa, błąd programisty (zła walidacja DTOsów)
2)Poniekąd racja, ale tutaj po prostu jest to wzglednie łatwe. Jeśli tworzymy pesel to wiadomo jaki String jest niepoprawny ;)
3)Tu się w ogóle nie zgodze. Either ma po prostu left i right i to już pozwala na łatwe sprawdzenie poprawności. Left pozwala na manewrowanie jakością komunikatu i tyle

Dopiero czytając ten temat dowiedziałem się, że istnieje taka biblioteka jak vavr. Ale po zagłębieniu się w jej dokumentację nasuwa się pytanie: dlaczego nie stosować Validation zamiast Either?

Either i Optional służy do czego innego niż assercje.
Assercja ValueObjectu to co innego niż Validacja punktu wejścia.
ValueObject powinien zawierć Assercje według Desing By Contract.
Tak mówi pismo święte.

@scibi92: rzuć okiem na tą książkę: https://www.manning.com/books/functional-programming-in-scala . Nie czytałem, ale na 62 i 213/214 stronie masz przykłady z walidacją za pomocą Either i Validation, może się przyda.

A może jeszcze inaczej:

public abstract class Pesel{

      private Pesel(String pesel){…}

      private static final class ValidPesel() extends Pesel{…}   
      private static final class InvalidPesel() extends Pesel{…}   

     public abstract boolean isValid();

      public static Pesel of(String pesel){
           PeselValidator.isValid(pesel)? new ValidPesel(pesel): new InvalidPesel(pesel);
      }
}

PESEL może być poprawny, pomimo że nie spełnia warunków walidacji (taki jego urok). Zwracasz zatem obiekt Pesel, który ma w środku informację o swojej poprawności. Następnie tam, gdzie wymagane jest by pesel był poprawny, odczytujesz tą informację i decydujesz co z nią zrobić.

Dlaczego nie:

1. Wyjątki – bo niepoprawne dane wejściowe nie są czymś wyjątkowym. Szczególnie w kontekście biznesowym.
2. Optional – bo Empty jest w tym przypadku dwuznaczne. Nie masz tak naprawdę informacji o błędzie, a jedynie słabe założenie w kodzie, że Empty reprezentuje błąd.
3. Either – to jest kontener, który nie powinien wypływać do biznesu. Jego rolę może przejąć para Valid/InvalidPesel, a jeżeli będą one publiczne i finalne, to można już zrobić coś w rodzaju pattern matchingu przez switch na stringach (brzydko) albo przez wywołania polimorficzne.

@Koziołek: tylko to jest trochę specyficzny przypadek (Pesel). Co w sytuacji gdy dla danych danych nie można utworzyć obiektu w ogóle?

2 i 3 pkt: Czym w tym przypadku różni się Optional i Either, że pierwszego można używać, a drugiego nie? Sam kontener nie musi wypływać do biznesu, a jedynie być efektem akcji "na" domenie.

@danek: jeżeli nie możesz utworzyć obiektu, to użyj odpowiedniego domenowego odpowiednika Either.

Czym w tym przypadku różni się Optional i Either, że pierwszego można używać, a drugiego nie?

To są dwa różne kontenery. Rozpatrzmy taki przypadek:

Optional<VO> of(String rawData){

    if(rawData == null) return Optional.empty();

    if(validator.validate(rawData))
        return Optional.of(new VO(rawData));
    return Optional.empty();
}

Pytanie, czy jesteś w stanie odpowiedzieć na pytanie, czy Empty oznacza prawidłowy stan – brak wartości, czy też stan błędną wartość?

Either pozwala na rozróżnienie tych dwóch stanów:

Either<VO, Error> of(String rawData){

    if(rawData == null) return Either.left();

    if(validator.validate(rawData))
        return Either.left(new VO(rawData));
    return Either.right(new ValidationError());
}

przy czym w tym przypadku zakładamy, że Either może przyjąć null jako prawidłową wartość.

Wyjątki – bo niepoprawne dane wejściowe nie są czymś wyjątkowym. Szczególnie w kontekście biznesowym.

W kontekście ValueObject niepoprawne dane wejściowe są czymś wyjątkowym, ponieważ nie powinny się tam znaleźć.

Mówi o tym rozdział 9 i 10 niebieskiej księgi według Erica Evansa.

@WyznawcaDDD: ale my mówimy o walidacji, czyli krok przed VO. O tym, jak elegancko powiadomić domenę, że VO nie może być utworzony. Jeżeli później okaże się, że VO zawiera niepoprawne dane, to niech leci nawet System.exit(0).