Validacja danych w konstruktorze

    private String imie;
    private String nazwisko;
    private String pesel;
    private Date dataUrodzenia;

    public Osoba(String imie, String nazwisko, String pesel, Date dataUrodzenia) {
        this.imie = imie;
        this.nazwisko = nazwisko;
        this.pesel = pesel;
        this.dataUrodzenia = dataUrodzenia;
    }

Mam taka najprostsza klase, chcialbym od razu przy tworzeniu obiektu uniemozliwic wprowadzenie innych wartosci niz np, pesel skladajacy sie z samych cyfr, 11 znakowy, ograniczyc date urodzenia.

Jedyny pomysl jaki mi przychodzi do glowy to sprawdzenie ifem
if (pesel.length() == 11) && (tutaj warunek zeby wszystkie znaki byly cyframi, ale da sie to zrobic bez iterowania po wszystkich indeksach?)
Ale wtedy i tak obiekt sie stworzy, tylko z nullem zamiast danej wartosci, a chcialbym juz przy deklarowaniu to uniemozliwic.

I jeszcze jakim typem danych przechowywac date urodzenia? Stworzyc do tego nowa klase z polami dzien miesiac rok?

Dzieki

Niech cię wszyscy święci bronią przed "starą" Date, użyj java.time.LocalDate, która weszła z Javą 8

Co zrobić, jak dane są złe? Rzucić wyjątkiem, wtedy obiekt się skutecznie nie utworzy. Na przykład

 throw new IllegalArgumentException("Błędny PESEL")

Tu zagadnienie filozoficzno-urzędniczo-projektowe, np obcokrajowiec w PL, dopóki się nie "zadomowi", peselu nie ma. Niektóre "piony" urzędowe pouczają, żeby podawać to czy tamto.

lukaswz napisał(a):

if (pesel.length() == 11) && (tutaj warunek zeby wszystkie znaki byly cyframi, ale da sie to zrobic bez iterowania po wszystkich indeksach?)

 boolean ok = pesel.chars().allMatch(Character::isDigit);  // Java >=8 

Fajnie jak obiekt nie pozwala utworzyć się w niedozwolonym stanie, natomiast pakowanie złożonej logiki walidacji do konstruktora jest nie OK z punktu widzenia testowalności i... obiektowości? Skomplikowaną walidacje wyrzuciłbym do jakiejś fabryki, która tworzy obiekty danej klasy. Co to znaczy „skomplikowaną” - kwestia gustu.

A ja bym jednak użył klasycznego wzorca Factory -> package konstruktor w klasie i Factory które pozwala stworzyć jedynie odpowiednio przygotowane obiekty i zwraca jakiś Either<ValidationError, T>. Alternatywnie można też Buildera, w podobnym guście.

import java.time.LocalDate;
import java.time.Year;
import java.util.Scanner;

public class Osoba {

    private int id;
    private String imie;
    private String nazwisko;
    private String pesel;
    private java.time.LocalDate dataUrodzenia;

    private Osoba(int id, String imie, String nazwisko, String pesel, java.time.LocalDate dataUrodzenia) {
        this.id = id;
        this.imie = imie;
        this.nazwisko = nazwisko;
        this.pesel = pesel;
        this.dataUrodzenia = dataUrodzenia;
    }

    public static class Factory {
        private static int id = 0;

        public static Osoba stworzOsobe(String imie, String nazwisko, String pesel, int dzienUrodzenia, int miesiacUrodzenia, int rokUrodzenia) {
            LocalDate dataUrodzenia = LocalDate.of(rokUrodzenia, miesiacUrodzenia, dzienUrodzenia);

            return new Osoba(id++, imie, nazwisko, pesel, dataUrodzenia);
        }

        public static Osoba stworzOsobe(){
            String imie = getString(15, "imię");
            String nazwisko = getString(20, "nazwisko");
            String pesel = getPesel();
            int dzienUrodzenia = getInt(1, 31, "dzien urodzenia");
            int miesiacUrodzenia = getInt(1, 12, "miesiac urodzenia");
            int rokUrodzenia = getInt(Year.now().getValue() - 110, Year.now().getValue(), "rok urodzenia");

            return stworzOsobe(imie, nazwisko, pesel, dzienUrodzenia, miesiacUrodzenia, rokUrodzenia);
        }

        private static int getInt(int min, int max, String nazwa) {
            Scanner s = new Scanner(System.in);
            System.out.println("Wpisz " + nazwa + ": ");
            while(true) {
                int input = s.nextInt();
                if(input <= max && input >= min)
                    return input;
            }
        }

        private static String getString(int dlugosc, String string) {
            Scanner s = new Scanner(System.in);
            System.out.println("Wpisz " + string + ": ");
            while(true) {
                String input = s.nextLine();
                if(input.length() <= dlugosc)
                    return input;
            }
        }

        private static String getPesel() {
            Scanner s = new Scanner(System.in);
            System.out.println("Wpisz pesel: ");
            while(true) {
                try {
                    String input = s.nextLine();
                    boolean ok = input.chars().allMatch(Character::isDigit) && input.length() == 11;
                    if (ok)
                        return input;
                    else throw new IllegalArgumentException("Błędny PESEL");
                } catch(IllegalArgumentException e) {
                    System.out.println("Błędny pesel");
                }
            }
        }
    }


    @Override
    public String toString() {
        return id + ". " + imie + " " + nazwisko + ", pesel: " + pesel + ", data urodzenia: " + dataUrodzenia;
    }
}

Skleilem coś takiego, ale zdaje sobie sprawe ze nie jest to najlepsze.

Najpierw dałem całą validacje do metod, ktore pobieraja dane z klawiatury, ale teraz doszedlem do wniosku, ze chce miec validacje tez przy tworzeniu osob z 'maina' (nie wiem jak to prawidlowo okreslic). Najbardziej prawidlowo by bylo gdybym potworzyl metody isValid dla inta, stringa i peselu?

I te validacje zrobilem najpierw ifami, bo myslalem ze przy uzyciu wyjatkow zawsze program bedzie sie zamykal, ale w peselu jakos to sie udalo, tylko to pewnie nie jest to o co chodzi. Moglby ktos nakreslic bardziej jak to prawidlowo powinno wygladac?

Dzieki

Widzę że korzystasz z uwag, dobrze to wygląda!

W mojej intuicji by wolał centralną walidację (czyli jakoś zintegriowaną z klasą, a nie z atomami, ale mogą być różne ujęcia). Na przykład dane są prawidłowe lub nie w kontekście innych danych. PESEL męski / żeński czy inne, jakby projekt rozwijać.

Lokalne catchowanie wyjątku mi się nie widzi, ... taaaak, to chyba jedyny poważny zarzut. .

W duchu Javy nie ma sensu isValid(), na rzecz gwarancji, że błędny obiekt nigdy nie powstanie. (choć są naruszenia tej zasady w nieładnych frameworkach).
I właśnie u Ciebie zbyt wczesne / w niewłaściwym miejscu łapanie wyjątku demontuje mechanizm tej gwarancji.

Niech koledzy proponujący fabrykę (to może być dobry pomysł) doradzą , jak fajnie to łapać.

lukaswz napisał(a):

ale teraz doszedlem do wniosku, ze chce miec validacje tez przy tworzeniu osob z 'maina' (nie wiem jak to prawidlowo okreslic).

Ja by użył słow "tworzenie przez usera" "tworzenie z konsoli/GUI" / "tworzenie z kodu".

Najbezpieczniej i "koszernie" - choć kosztem sporej ilości kodu, w każdym razie w Javie - można to zrobić zaprzęgając do tego system typów.

Czyli można zrobić walidator (teoretycznie mógłby być "wciągnięty" do fabryki i nie stanowić osobnej klasy, jeśli chcemy iść na skróty):

public class PeselValidator {
    public boolean isValid(String pesel) {
        return pesel.length() == 11; // plus inne weryfikacje właściwe dla PESEL-u, np. cyfry kontrolnej itp.
    }
}

A fabryka:

public final class PeselFactory {
    private final PeselValidator validator;

    public PeselFactory(PeselValidator validator) {
        this.validator = validator;
    }

    abstract class Pesel {
        final String value;

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

    final class ValidPesel extends Pesel {
        private ValidPesel(String value) {
            super(value);
        }
    }

    final class InvalidPesel extends Pesel {
        private InvalidPesel(String value) {
            super(value);
        }
    }
    
    public Pesel createPesel(String pesel) {
        return validator.isValid(pesel) 
                ? new ValidPesel(pesel) 
                : new InvalidPesel(pesel);
    }
}

Czyli, w duchu programowania funkcyjnego, prawidłowa wartość PESEL staje się konkretnym typem, ValidPesel. (W Kotlinie można by to zaimplementować bardziej elegancko dzięki sealed classes).

Ponieważ klasa ta ma prywatny konstruktor, nie jesteś w stanie stworzyć jej inaczej niż poza tą fabryką, czyli nie ma jak obejść walidatora.

Od tej pory klasa Person (czy tam Osoba) nie przyjmuje wartości PESEL w formie prymitywnego typu String, ale ValidPesel pesel.

Dzięki temu, z uwagi na powyższe, fizycznie nie da się do niej przekazać nieprawidłowego PESELu. Nie blokujemy tego wyjątkiem (który działa dopiero w czasie uruchomienia), ale na poziomie silnika typów, czyli już na etapie kompilacji.

Zaletą jest ściągnięcie walidacji z poziomu Osoby do PESEL-u, który staje się pełnoprawnym obiektem. Ma to spory plus: bo niby dlaczego to Osoba ma wiedzieć, co to jest prawidłowy PESEL?

Jeśli pojawi się potrzeba walidacji innych danych, np. NIP-u albo daty urodzenia, również zamkniemy tę logikę w osobnych typach. Dzięki temu unikniemy pakowania w kod zajmujący się Osobami ogólnie - wiedzy na temat różnych szczegółów biurokratycznych.

A PESEL można odtąd walidować (i testować tę walidację) także poza kontekstem Osoby.

V-2 napisał(a):

Najbezpieczniej i "koszernie" - choć kosztem sporej ilości kodu, w każdym razie w Javie - można to zrobić zaprzęgając do tego system typów.
...

W mojej opinii jest to prawie dobry sposób na zamodelowanie tego problemu. Główny mankament jaki widzę to fakt, że zarówno ValidPesel jak i InvalidPesel dziedziczą po Pesel. Przez to, zostaje udostępniona możliwość napisania funkcji, która przyjmuje Pesel i walidnie przekazać InvalidPesel. Więc niby jest wykorzystany system typów, a jednak nadal pozwala na napisanie potencjalnie niebezpiecznej funkcji.

Żeby być bezpieczniejszym, polecałbym zamodelowanie błędów walidacji jako osobny typ (nie dziedziczący po Pesel), a w fabryce jako rezultat zwracać Either<PeselValidationError, Pesel>. Dzięki temu łatwo można rozszerzać PeselValidationError o kolejne przypadki walidacji, a jednocześnie eliminujesz użycie niepoprawnej wartości w innych funkcjach. To z kolei powoduje, że w całym systemie istnieje tylko jeden konkretny typ reprezentuje walidny Pesel

DisQ napisał(a):

W mojej opinii jest to prawie dobry sposób na zamodelowanie tego problemu. Główny mankament jaki widzę to fakt, że zarówno ValidPesel jak i InvalidPesel dziedziczą po Pesel. Przez to, zostaje udostępniona możliwość napisania funkcji, która przyjmuje Pesel i walidnie przekazać InvalidPesel. Więc niby jest wykorzystany system typów, a jednak nadal pozwala na napisanie potencjalnie niebezpiecznej funkcji.

Mogą przecież być scenariusze, w których chcemy przetwarzać niezwalidowaną wartość PESEL. Walidacja może być w jakiś sposób odroczona, następować w innej warstwie kodu. Np. użytkownik na razie dodaje dane różnych osób hurtem, albo są zaciągane z jakiejś istniejącej już wcześniej bazy, której zawartości - pod względem prawidłowości - nie ufamy. Dla jasności można nazwać tę klasę bazową UnvalidatedPesel - taki kot Schrödingera.

Żeby być bezpieczniejszym, polecałbym zamodelowanie błędów walidacji jako osobny typ (nie dziedziczący po Pesel), a w fabryce jako rezultat zwracać Either<PeselValidationError, Pesel>.

To oczywiście jeszcze bezpieczniejsze, zgadzam się. Nie chciałem rzucać @lukaswz na zbyt głęboką wodę :)

lukaswz napisał(a):

I jeszcze jakim typem danych przechowywac date urodzenia? Stworzyc do tego nowa klase z polami dzien miesiac rok?

Stary, włożyłeś kij w mrowisko.

Jak zadasz takie pytanie tutaj, i ktoś wysunie pomysł który działa w 98%, to zaraz się znajdą ludzie którzy zaczną wytykać czemu to rozwiazanie ssie, bo nie działa w 2%.

Ja osobiście, zrobiłbym coś takiego

class Pesel {
  Pesel(String number) {
    if (!PeselFormat.isValid(number)) throw new IllegalArgumentException();
    this.number = number;
  }
}

class PeselFormat {
  boolean static isValid(String number) {
    // tutaj walidacjia peselu
  }
}

I wtedy klasy PeselFormat można użyć zarówno do tworzenia new Pesel(), jak i np w UI do walidacji na poziomie interfejsu użytkownika bądź w innych miejscach, nawet jak nie potrzebujesz obiektu Pesel.

Poza tym, zauważ ze walidacja walidacji nie równa. Np czy pesel osoby która urodziła się w 2022 jest poprawny? Z punktu widzenia formatu - tak. Z punktu widzenia działania aplikacji - zależy. Więc moim zdaniem klas walidacji powinno być więcej, zależnie od potrzeb. Umieszczanie jednej zbyt restrykcyjnej w konstruktorze wydaje się dodatkowym utrudnieniem.

V-2 napisał(a):

Mogą przecież być scenariusze, w których chcemy przetwarzać niezwalidowaną wartość PESEL. Walidacja może być w jakiś sposób odroczona, następować w innej warstwie kodu. Np. użytkownik na razie dodaje dane różnych osób hurtem, albo są zaciągane z jakiejś istniejącej już wcześniej bazy, której zawartości - pod względem prawidłowości - nie ufamy. Dla jasności można nazwać tę klasę bazową UnvalidatedPesel - taki kot Schrödingera.

Jeśli walidacja miałaby być odroczona z jakiegoś powodu to pewnie tak jak mówisz, utworzyłbym zupełnie inny typ który jasno wskazuje na to, że Pesel nie jest zwalidowany i może posiadać niepoprawną wartość. Im więcej constraintów uda się zawrzeć w typach tym lepiej ;)

Tak mi się klasyk przypomniał ;)

Szukałem kiedyś latarki. Tzw. czołówki. Na jeden wyjazd z namiotem na 3 dni. Konkretnie to googlałem gdzie jest jakaś marketowa promocja, żeby zamiast 23zł zapłacić coś bliżej 10. Okazuje się, że latarki mają forum. Na forum dowiedziałem się, że najtańsza latarka która świeci, kosztuje 200zł. Ale to i tak badziew, bo w razie jakbym wpadł do jaskini i czekał dwa tygodnie na pomoc, to wodę mogę lizać ze ścian, ale latarka zgaśnie mi po 7 dniach nieustannego świecenia. A to przecież absolutnie nieakceptowalne. Tańsze w ogóle nie wchodzą w grę, bo przecież przy 100 lumenach zgubię się w ciemności. Będę widział tylko na 5 metrów zamiast na kilometr. Na dodatek nie da się tańszych w ogóle używać, bo kąt świecenia w każdym egzemplarzu różni się o parę stopni, czyli to musi być badziew. Całkowicie dyskwalifikujący jest fakt że raz w roku będę musiał zmienić baterię, a w wypadku gdybym wpadł do oceanu to przeciekają po godzinie. Mieli też dział DYI w którym pokazywali np jak użyć sprzętu pomiarowego za 5tys zł żeby ustalić jak zmienić fabryczny rezystorek coby latarka miała pół lumena więcej. I do tego odpowiedzi na kilkanaście stron z gratulacjami i bluzgami na producenta że sam na to nie wpadł. Latarkę kupiłem w końcu za 9zł w pierwszym lepszym markecie. Nie wpadłem do jaskini ani do oceanu a baterie wymieniłem po roku.

V-2 napisał(a):

Tak mi się klasyk przypomniał ;)

Rozumiem że ma to być zabawne, ale imo nie jest. Im bardziej zagłębiasz się w temat tym większych standardów oczekujesz od siebie i od swoich narzędzi.

Szukać latarek za 9zł na forum o latarkach to jak szukać na 4programmers.net tutoriala do goto, za pewne.

TomRiddle napisał(a):

V-2 napisał(a):

Tak mi się klasyk przypomniał ;)

Im bardziej zagłębiasz się w temat tym większych standardów oczekujesz od siebie i od swoich narzędzi.

Dziękuję za tę oświecającą uwagę - wiem, że sam Papa Smerf zawsze to powtarza ;) Zajmuję się tym fachem - podejrzewam - dłużej niż większość na tym forum, toteż nie trzeba mi tego tłumaczyć. I czy ja mówię, że tak nie jest?

Wczuwam się tylko w psychiczną sytuację nowicjusza, który pewnie nawet nie podejrzewał, że jak się spyta fachowców, wyrośnie przed nim nagle tak wysoka drabina standardów i oto przechodzi ekspresowy kurs z OOD, na wylotówce do FP. A dopiero co biedził się z grzebaniem w konstruktorach :)