Walidacja danych przy tworzeniu obiektów

Jak najlepiej zadbać o poprawność tworzonych obiektów? Czy sposób z poniższego przykładu byłby okej jeśli chodzi o best practices?

    public class Category
    {
        public int Id { get; private set; }
        public string Name { get; private set; }

        private Category(string name)
        {
             Name = name;
        }
    
        public static Result<Category> Create(string name)
        {
            if (string.IsNullOrEmpty(name))
                return new ValidationError("Category should contain at least 3 characters.");

            return new Category(name);
        }
    }

Może lepiej inaczej do tego podejść?

Pytanie, czy to obiekt domenowy, czy jakieś DTO, czy jeszcze coś innego?

Jeśli domenowy, to rzucałbym wyjątki z konstruktora. Na etapie tworzenia obiektu domenowego nie powinno być nieprawidłowych danych, a więc taka sytuacja jest wyjątkiem.
Jeśli DTO, to użyłbym FluentValidation do jego zweryfikowania.
Nie mówię, że Twoje podejście jest złe, po prostu nie wiem gdzie ono żyje. ;) W sensie - w jakim miejscu systemu masz obiekty, których właściwości nie chcesz ustawiać z zewnątrz, ale jednocześnie dane do ich utworzenia mogą być niepoprawne?

somekind napisał(a):

Pytanie, czy to obiekt domenowy, czy jakieś DTO, czy jeszcze coś innego?

To obiekt domenowy (reprezentuje kategorię produktu), który tworzę w serwisie na podstawie DTO, który to przekazywany jest z kontrolera np. (CategoryCreateDTO).

Czyli sugerujesz, że najpierw powinienem sobie w serwisie zweryfikować poprawność danych w DTO i w razie błędu zwrócić komunikat o błędzie. Następnie na jego [DTO] podstawie tworzyć sobie obiekt Category, który potem sobie np. zapiszę do bazy danych? Czyli w klasie Category i tak powinienem powtórzyć sprawdzanie poprawność danych, z tymże rzucając wyjątki w razie nieprawidłowości (bo jakimś cudem wartość będzie niepoprawna)?

public class Category
    {
        public int Id { get; private set; }
        public string Name { get; private set; }

        public Category(string name)
        {
             if (string.IsNullOrEmpty(name))
                throw new ArgumentException(nameof(name), ...);

            Name = name
        }
    }

twoj_stary napisał(a):

Czyli sugerujesz, że najpierw powinienem sobie w serwisie zweryfikować poprawność danych w DTO i w razie błędu zwrócić komunikat o błędzie.

Tak, tylko nie bezpośrednio w serwisie lecz w jakimś wydzielonym walidatorze. No i też, żeby nie wynajdować koła na nowo lepiej użyć sprawdzonej biblioteki.

Następnie na jego [DTO] podstawie tworzyć sobie obiekt Category, który potem sobie np. zapiszę do bazy danych? Czyli w klasie Category i tak powinienem powtórzyć sprawdzanie poprawność danych, z tymże rzucając wyjątki w razie nieprawidłowości (bo jakimś cudem wartość będzie niepoprawna)?

Dla bezpieczeństwa rzucałbym wyjątek, bo nigdy nie powinno się dać utworzyć zepsutego obiektu. A to może nastąpić, np.:

• walidacja się zepsuje, a nie jest dostatecznie pokryta testami;
• pojawi się nowy flow, w którym obiekt domenowy będzie tworzony nie z DTO, więc nie będzie uprzedniej walidacji.

Ogólnie, walidacja służy do tego, aby powiedzieć użytkownikowi, co zrobił źle, a niezależnie od niej, to obiekt powinien być sam w sobie zabezpieczony przed utworzeniem go w sposób, w którym nie będzie działał poprawnie.

PS. No i nie zapisywałbym obiektów domenowych do bazy danych. Chyba, że to jakiś anemiczny crud, a domena to tak naprawdę DTO. Ale to chyba nie ten przypadek, skoro masz konstruktory i nie masz setterów.

Data Annotations

https://docs.microsoft.com/pl-pl/aspnet/mvc/overview/older-versions-1/models-data/validation-with-the-data-annotation-validators-cs

A, dobrze, że ktoś o tym wspomniał.

Nie używaj DataAnnotations, bo to nie dość, ze działa jak kupa, to jeszcze tak wygląda.

somekind napisał(a):

Dla bezpieczeństwa rzucałbym wyjątek, bo nigdy nie powinno się dać utworzyć zepsutego obiektu. A to może nastąpić, np.:

• walidacja się zepsuje, a nie jest dostatecznie pokryta testami;
• pojawi się nowy flow, w którym obiekt domenowy będzie tworzony nie z DTO, więc nie będzie uprzedniej walidacji.

Ogólnie, walidacja służy do tego, aby powiedzieć użytkownikowi, co zrobił źle, a niezależnie od niej, to obiekt powinien być sam w sobie zabezpieczony przed utworzeniem go w sposób, w którym nie będzie działał poprawnie.

Okej, a co z metodami zmieniającymi stan obiektu? Dla przykładu dodałem metodę SetName, za pomocą której chcę móc zmieniać nazwę kategorii. W niej należałoby sprawdzać poprawność parametru name i zwracać jakiś Result, czy też rzucać wyjątkami?

 public class Category
    {
        public int Id { get; private set; }
        public string Name { get; private set; }
        public virtual ICollection<Product> Products { get; private set; }
    
        public Category(string name)
        {
            if (string.IsNullOrEmpty(name))
                throw new ArgumentException("Category name cannot be empty.", nameof(name));

            if (name.Length < 3)
                throw new ArgumentException("Category name is too short. Min. 3 characters long.", nameof(name));

            Name = name;
        }

        public Result<bool> SetName(string name)
        {
            if (string.IsNullOrEmpty(name))
                return new ValidationError("Category name cannot be empty.");
             
              if (name.Length < 3)
                return new ValidationError("Category name is too short. Min. 3 characters long.");

            Name = name;
            return true;
        }
    }

Tak samo jak w konstruktorze rzucałbym wyjątkiem. Skoro wywołujesz metodę logiki domenowej, to name powinieneś mieć już sprawdzone wcześniej, w warstwie czytającej z pliku, pobierającej od użytkownika, czy skądkolwiek to się bierze.

@somekind:

if (name.Length < 3)

no ale patrz, teraz w obiekcie domenowym masz jakąś logikę walidacji oraz osobną (pewnie taką samą) zdefiniowaną w jakiejś fluent walidacji

jak można byłoby sensownie tego użyć ponownie, aby nie namieszać w domenie jakimiś zależnościami, a nadal mieć "jedno źródło prawdy", co zapewniłoby spójność?

WeiXiao napisał(a):

no ale patrz, teraz w obiekcie domenowym masz jakąś logikę walidacji oraz osobną (pewnie taką samą) zdefiniowaną w jakiejś fluent walidacji

To nie jest logika walidacji, to jest spójność modelu. On jest jedynym źródłem prawdy, a walidacja w wyższych warstwach jest dla wygody. Podobnie jak z kolejną walidacją na frontendzie.

jak można byłoby sensownie tego użyć ponownie, aby nie namieszać w domenie jakimiś zależnościami, a nadal mieć "jedno źródło prawdy", co zapewniłoby spójność?

Sensownie to raczej nie, bo jak zaczniemy frontend generować z domeny, to wyjdzie jakiś anemiczny CRUD.

Najlepiej zrobić metodę CreateObj z parametrami zawierającą wszystkie walidacje

somekind napisał(a):

Nie używaj DataAnnotations, bo to nie dość, ze działa jak kupa, to jeszcze tak wygląda (...) Ja nigdy chyba nie miałem aż tak prostej walidacji.

Czemu działa jak kupa? I ciekawi mnie przykład tak skomplikowanej walidacji, żeby DataAnnotations sobie nie poradziły.

No to weźmy sobie taki zestaw DTO dla jakiegoś requestu:

class OrderRequest
{
    public List<OrderItem> Items { get; set; }
    public PaymentMethod Payment { get; set; }
    public DeliveryAddress DeliveryAddress { get; set; }
}

class OrderItem
{
    public string Name { get; set; }
    public decimal Price { get; set; }
}

abstract class PaymentMethod
{
}

class PaymentCard : PaymentMethod
{
    public string Network { get; set; }
    public string Number { get; set; }
    public string NameOnCard { get; set; }
}

class PayPalPayment : PaymentMethod
{
    public string Id { get; set; }
}

class DeliveryAddress
{
    public string Line1 { get; set; }
    public string Line2 { get; set; }
    public string Line3 { get; set; }
    public string Line3 { get; set; }
    public string PostalCode { get; set; }
    public string CountryCode { get; set; }
}

Akceptowane karty płatnicze to Visa, Visa Electron, Mastercard, Maestro, American Express i JCB, wszystkie inne trzeba odrzucić. Oczywiście numer karty musi być poprawny.
PayPal dostępny od kwoty zamówienia powyżej 100.00.
Jeśli adres należy do UE, to walidujemy też kod pocztowy, serwis do kodów pocztowych pod takim interfejsem: IPostalCodeProvider.GetForCountry(string iso3166alpha2).
W adresie Line1 i Line2 wymagane, Line3 nie, ale Line4 może być podane tylko wtedy, jeśli Line3 nie jest puste.
Poza tym wszystkie inne pola wymagane, kolekcje niepuste, itd.
Każdy błąd ma zwracać oprócz komunikatu także kod błędu.
No i nie dotykamy kodu naszych DTO, bo przecież nie chcemy ufajdać kodu DTO regułami walidacji.

Jak będzie wyglądało rozwiązanie w Data Annotations?