Czy istnieje łatwiejszy sposób na sprawdzenie poprawności argumentów i inicjalizacji pola w niezmiennym obiekcie?

20

Moja domena składa się z wielu prostych niezmiennych klas takich jak to:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        if (fullName == null)
            throw new ArgumentNullException(nameof(fullName));
        if (nameAtBirth == null)
            throw new ArgumentNullException(nameof(nameAtBirth));
        if (taxId == null)
            throw new ArgumentNullException(nameof(taxId));
        if (phoneNumber == null)
            throw new ArgumentNullException(nameof(phoneNumber));
        if (address == null)
            throw new ArgumentNullException(nameof(address));

        FullName = fullName;
        NameAtBirth = nameAtBirth;
        TaxId = taxId;
        PhoneNumber = phoneNumber;
        Address = address;
    }
}

Pisanie kontroli zerowej i inicjowanie właściwości jest już bardzo uciążliwe, ale obecnie piszę testy jednostkowe dla każdej z tych klas, aby sprawdzić, czy sprawdzanie poprawności argumentów działa poprawnie i czy wszystkie właściwości zostały zainicjowane. Wydaje się, że praca jest wyjątkowo nudna i przynosi niewspółmierne korzyści.

Prawdziwym rozwiązaniem byłoby, aby język C # obsługiwał niezmienność i typy zerowalne. Ale co mogę w międzyczasie poprawić? Czy warto pisać wszystkie te testy? Czy dobrym pomysłem byłoby napisanie generatora kodu dla takich klas, aby uniknąć pisania testów dla każdej z nich?


Oto, co mam teraz na podstawie odpowiedzi.

Mogę uprościć sprawdzanie wartości zerowej i inicjowanie właściwości, aby wyglądały następująco:

FullName = fullName.ThrowIfNull(nameof(fullName));
NameAtBirth = nameAtBirth.ThrowIfNull(nameof(nameAtBirth));
TaxId = taxId.ThrowIfNull(nameof(taxId));
PhoneNumber = phoneNumber.ThrowIfNull(nameof(phoneNumber));
Address = address.ThrowIfNull(nameof(address));

Za pomocą następującej implementacji Roberta Harveya :

public static class ArgumentValidationExtensions
{
    public static T ThrowIfNull<T>(this T o, string paramName) where T : class
    {
        if (o == null)
            throw new ArgumentNullException(paramName);

        return o;
    }
}

Testowanie czeków zerowych jest łatwe przy użyciu narzędzia GuardClauseAssertionfrom AutoFixture.Idioms(dzięki za sugestię, Esben Skov Pedersen ):

var fixture = new Fixture().Customize(new AutoMoqCustomization());
var assertion = new GuardClauseAssertion(fixture);
assertion.Verify(typeof(Address).GetConstructors());

Można to jeszcze bardziej skompresować:

typeof(Address).ShouldNotAcceptNullConstructorArguments();

Za pomocą tej metody rozszerzenia:

public static void ShouldNotAcceptNullConstructorArguments(this Type type)
{
    var fixture = new Fixture().Customize(new AutoMoqCustomization());
    var assertion = new GuardClauseAssertion(fixture);

    assertion.Verify(type.GetConstructors());
}
Botond Balázs
źródło
3
Tytuł / znacznik mówi o (jednostkowym) testowaniu wartości pól, co sugeruje jednostronne testowanie obiektu po zakończeniu budowy, ale fragment kodu pokazuje sprawdzanie argumentu / parametru wejściowego; to nie są te same pojęcia.
Erik Eidt
2
Do Twojej wiadomości, możesz napisać szablon T4, który ułatwiłby tego rodzaju płytę kotłową. (Możesz również wziąć pod uwagę string.IsEmpty () poza == null.)
Erik Eidt
1
Przyjrzałeś się idiomom autofixture? nuget.org/packages/AutoFixture.Idioms
Esben Skov Pedersen
1
Powiązane: stackoverflow.com/questions/291340/…
Doc Brown
1
Możesz także użyć Fody / NullGuard , choć wygląda na to, że nie ma domyślnego trybu zezwolenia.
Bob

Odpowiedzi:

5

Stworzyłem szablon t4 właśnie dla tego rodzaju przypadków. Aby uniknąć pisania dużej liczby płyt dla klas Niezmiennych.

https://github.com/xaviergonz/T4Immutable T4Immutable to szablon T4 dla aplikacji C # .NET, który generuje kod dla klas niezmiennych.

Mówiąc konkretnie o testach niepustych, jeśli użyjesz tego:

[PreNotNullCheck, PostNotNullCheck]
public string FirstName { get; }

Konstruktor będzie taki:

public Person(string firstName) {
  // pre not null check
  if (firstName == null) throw new ArgumentNullException(nameof(firstName));

  // assignations + PostConstructor() if needed

  // post not null check
  if (this.FirstName == null) throw new NullReferenceException(nameof(this.FirstName));
}

Powiedziawszy to, jeśli używasz Adnotacji JetBrains do sprawdzania wartości NULL, możesz również to zrobić:

[JetBrains.Annotations.NotNull, ConstructorParamNotNull]
public string FirstName { get; }

Konstruktor będzie taki:

public Person([JetBrains.Annotations.NotNull] string firstName) {
  // pre not null check is implied by ConstructorParamNotNull
  if (firstName == null) throw new ArgumentNullException(nameof(firstName));

  FirstName = firstName;
  // + PostConstructor() if needed

  // post not null check implied by JetBrains.Annotations.NotNull on the property
  if (this.FirstName == null) throw new NullReferenceException(nameof(this.FirstName));
}

Jest też kilka innych funkcji niż ta.

Javier G.
źródło
Dziękuję Ci. Właśnie próbowałem i zadziałało idealnie. Zaznaczam to jako zaakceptowaną odpowiedź, ponieważ dotyczy ona wszystkich problemów w pierwotnym pytaniu.
Botond Balázs
16

Możesz uzyskać trochę poprawy dzięki prostemu refaktoryzacji, która może złagodzić problem pisania wszystkich tych ogrodzeń. Najpierw potrzebujesz tej metody rozszerzenia:

internal static T ThrowIfNull<T>(this T o, string paramName) where T : class
{
    if (o == null)
        throw new ArgumentNullException(paramName);

    return o;
}

Następnie możesz napisać:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        FullName = fullName.ThrowIfNull(nameof(fullName));
        NameAtBirth = nameAtBirth.ThrowIfNull(nameof(nameAtBirth));
        TaxId = taxId.ThrowIfNull(nameof(taxId));
        PhoneNumber = phoneNumber.ThrowIfNull(nameof(fullName));
        Address = address.ThrowIfNull(nameof(address));
    }
}

Zwrócenie oryginalnego parametru w metodzie rozszerzenia tworzy płynny interfejs, więc możesz rozszerzyć tę koncepcję o inne metody rozszerzenia, jeśli chcesz, i połączyć je wszystkie razem w zadaniu.

Inne techniki są bardziej eleganckie w koncepcji, ale stopniowo coraz bardziej skomplikowane w wykonaniu, takie jak dekorowanie parametru [NotNull]atrybutem i używanie tego typu Refleksji .

To powiedziawszy, możesz nie potrzebować wszystkich tych testów, chyba że twoja klasa jest częścią publicznego API.

Robert Harvey
źródło
3
Dziwne, że zostało to odrzucone. Jest bardzo podobny do podejścia podanego w najbardziej uprzywilejowanej odpowiedzi, z tym że nie zależy od Roslyn.
Robert Harvey
Nie podoba mi się to, że jest podatny na modyfikacje konstruktora.
jpmc26,
@ jpmc26: Co to znaczy?
Robert Harvey,
1
Gdy czytam twoją odpowiedź, sugeruje to, że nie testujesz konstruktora , ale tworzysz wspólną metodę wywoływaną dla każdego parametru i testujesz ją. Jeśli dodasz nową parę właściwość / argument i zapomnisz przetestować nullargument dla konstruktora, nie otrzymasz testu zakończonego niepowodzeniem.
jpmc26,
@ jpmc26: Naturalnie. Ale to również prawda, jeśli napiszesz to w konwencjonalny sposób, jak pokazano w PO. Jedyną powszechną metodą jest przenoszenie throwzewnętrznej części konstruktora; tak naprawdę nie przenosisz kontroli w konstruktorze gdzieś indziej. To tylko użyteczna metoda i sposób płynnego robienia rzeczy , jeśli tak zdecydujesz.
Robert Harvey,
7

W krótkim okresie nie można wiele zrobić w kwestii żmudności pisania takich testów. Istnieje jednak pewna pomoc związana z wyrażeniami rzutowania, które zostaną zaimplementowane w ramach następnej wersji C # (v7), prawdopodobnie w ciągu najbliższych kilku miesięcy:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        FullName = fullName ?? throw new ArgumentNullException(nameof(fullName));
        NameAtBirth = nameAtBirth ?? throw new ArgumentNullException(nameof(nameAtBirth));
        TaxId = taxId ?? throw new ArgumentNullException(nameof(taxId)); ;
        PhoneNumber = phoneNumber ?? throw new ArgumentNullException(nameof(phoneNumber)); ;
        Address = address ?? throw new ArgumentNullException(nameof(address)); ;
    }
}

Możesz eksperymentować z wyrażeniami rzutowania za pośrednictwem aplikacji internetowej Try Roslyn .

David Arno
źródło
O wiele bardziej kompaktowy, dziękuję. Połowa tyle linii. Czekamy na C # 7!
Botond Balázs
@ BotondBalázs możesz wypróbować teraz w podglądzie VS15, jeśli chcesz
user1306322
7

Dziwi mnie, że nikt jeszcze nie wspomniał o NullGuard.Fody . Jest dostępny za pośrednictwem NuGet i automatycznie wplata te kontrole zerowe w IL podczas kompilacji.

Więc twój kod konstruktora byłby po prostu

public Person(
    string fullName,
    string nameAtBirth,
    string taxId,
    PhoneNumber phoneNumber,
    Address address)
{
    FullName = fullName;
    NameAtBirth = nameAtBirth;
    TaxId = taxId;
    PhoneNumber = phoneNumber;
    Address = address;
}

a NullGuard doda te kontrole zerowe za przekształcenie go w dokładnie to, co napisałeś.

Zauważ jednak, że NullGuard jest opt-out , to znaczy doda te kontrole zerowe do każdej metody i argumentu konstruktora, gettera właściwości i ustawiacza, a nawet zwróci wartości metod, chyba że wyraźnie zezwolisz na wartość zerową z tym [AllowNull]atrybutem.

Roman Reiner
źródło
Dziękuję, to wygląda na bardzo miłe ogólne rozwiązanie. Chociaż jest to bardzo niefortunne, że domyślnie modyfikuje zachowanie języka. Chciałbym, żeby było o wiele więcej, gdyby działało, dodając [NotNull]atrybut zamiast nie pozwalając, aby wartość domyślna to null.
Botond Balázs
@ BotondBalázs: PostSharp ma to wraz z innymi atrybutami sprawdzania poprawności parametrów. W przeciwieństwie do Fody'ego nie jest jednak darmowy. Wygenerowany kod będzie również wywoływał biblioteki DLL PostSharp, więc musisz dołączyć je do swojego produktu. Fody będzie uruchamiał swój kod tylko podczas kompilacji i nie będzie wymagany w czasie wykonywania.
Roman Reiner
@ BotondBalázs: Na początku wydawało mi się również dziwne, że NullGuard stosuje się domyślnie, ale to naprawdę ma sens. W każdej rozsądnej bazie kodu dopuszczenie wartości null powinno być znacznie mniej powszechne niż sprawdzanie wartości null. Jeśli naprawdę chcesz gdzieś wyzerować, podejście opt-out zmusza cię do bardzo dokładnego podejścia.
Roman Reiner
5
Tak, to rozsądne, ale narusza zasadę najmniejszego zdziwienia . Jeśli nowy programista nie wie, że projekt używa NullGuard, czeka go wielka niespodzianka! Nawiasem mówiąc, nie rozumiem również opinii negatywnej, jest to bardzo przydatna odpowiedź.
Botond Balázs
1
@ BotondBalázs / Roman, wygląda na to, że NullGuard ma nowy tryb jawny, który zmienia domyślny sposób działania
Kyle W
5

Czy warto pisać wszystkie te testy?

Nie, prawdopodobnie nie. Jakie jest prawdopodobieństwo, że to spieprzysz? Jakie jest prawdopodobieństwo, że niektóre semantyki zmienią się spod ciebie? Jaki wpływ ma ktoś, kto to spieprzy?

Jeśli spędzasz dużo czasu na testowaniu czegoś, co rzadko się psuje, i jest to trywialna poprawka, jeśli to zrobiło ... może nie było tego warte.

Czy dobrym pomysłem byłoby napisanie generatora kodu dla takich klas, aby uniknąć pisania testów dla każdej z nich?

Może? Tego rodzaju rzeczy można łatwo zrobić za pomocą refleksji. Należy wziąć pod uwagę generowanie kodu dla prawdziwego kodu, więc nie masz N klas, które mogą zawierać błąd ludzki. Zapobieganie błędom> wykrywanie błędów.

Telastyn
źródło
Dziękuję Ci. Może moje pytanie nie było wystarczająco jasne, ale chciałem napisać generator, który generuje niezmienne klasy, a nie testy dla nich
Botond Balázs,
2
Widziałem też kilka razy zamiast tego if...throw, co mają ThrowHelper.ArgumentNull(myArg, "myArg"), w ten sposób logika jest nadal obecna i nie drażni Cię zasięg kodu. Gdzie ArgumentNullmetoda zrobi if...throw.
Matthew
2

Czy warto pisać wszystkie te testy?

Nie.
Ponieważ jestem pewien, że przetestowałeś te właściwości za pomocą innych testów logiki, w których używane są te klasy.

Na przykład możesz mieć testy dla klasy Factory, które mają asercje oparte na tych właściwościach ( Namena przykład wystąpienie utworzone z odpowiednio przypisaną właściwością).

Jeśli te klasy zostaną udostępnione publicznemu interfejsowi API, który jest używany przez jakiegoś trzeciego użytkownika / użytkownika końcowego (@EJoshua dziękuję za uwagę), testy na oczekiwane ArgumentNullExceptionmogą być przydatne.

Podczas oczekiwania na C # 7 możesz użyć metody rozszerzenia

public MyClass(string name)
{
    name.ThrowArgumentNullExceptionIfNull(nameof(name));
}

public static void ThrowArgumentNullExceptionIfNull(this object value, string paramName)
{
    if(value == null)
        throw new ArgumentNullException(paramName);
}

Do testowania można użyć jednej sparametryzowanej metody testowej, która używa odbicia do utworzenia nullodniesienia dla każdego parametru i potwierdzenia oczekiwanego wyjątku.

Fabio
źródło
1
To przekonujący argument za nie testowaniem inicjalizacji właściwości.
Botond Balázs
To zależy od tego, jak korzystasz z kodu. Jeśli kod jest częścią biblioteki publicznej, nigdy nie powinieneś ślepo ufać innym osobom, które wiedzą, jak masz bibliotekę (szczególnie jeśli nie mają dostępu do kodu źródłowego); Jeśli jednak jest to coś, czego używasz wewnętrznie, aby Twój własny kod działał, powinieneś wiedzieć, jak korzystać z własnego kodu, więc kontrole mają mniejszy cel.
EJoshuaS - Przywróć Monikę
2

Zawsze możesz napisać następującą metodę:

// Just to illustrate how to call this
private static void SomeMethod(string a, string b, string c, string d)
    {
        ValidateArguments(a, b, c, d);
        // ...
    }

    // This is the one to use as a utility function
    private static void ValidateArguments(params object[] args)
    {
        for (int i = 0; i < args.Length; i++)
        {
            if (args[i] == null)
            {
                StackTrace trace = new StackTrace();
                // Get the method that called us
                MethodBase info = trace.GetFrame(1).GetMethod();

                // Get information on the parameter that is null so we can add its name to the exception
                ParameterInfo param = info.GetParameters()[i];

                // Raise the exception on behalf of the caller
                throw new ArgumentNullException(param.Name);
            }
        }
    }

Przynajmniej zaoszczędzi Ci to trochę pisania, jeśli masz kilka metod, które wymagają tego rodzaju sprawdzania poprawności. Oczywiście to rozwiązanie zakłada, że żaden z parametrów twojej metody nie może być null, ale możesz to zmodyfikować, aby to zmienić, jeśli chcesz.

Można to również rozszerzyć, aby przeprowadzić inną weryfikację specyficzną dla typu. Na przykład, jeśli masz regułę, że ciągi nie mogą być wyłącznie białymi znakami lub puste, możesz dodać następujący warunek:

// Note that we already know based on the previous condition that args[i] is not null
else if (args[i].GetType() == typeof(string))
            {
                string argCast = arg as string;

                if (!argCast.Trim().Any())
                {
                    ParameterInfo param = GetParameterInfo(i);

                    throw new ArgumentException(param.Name + " is empty or consists only of whitespace");
                }
            }

Metoda GetParameterInfo, do której się odwołuję, w zasadzie robi odbicie (aby nie musiałem ciągle powtarzać tego samego, co byłoby naruszeniem zasady DRY ):

private static ParameterInfo GetParameterInfo(int index)
    {
        StackTrace trace = new StackTrace();

        // Note that we have to go 2 methods back to get the ValidateArguments method's caller
        MethodBase info = trace.GetFrame(2).GetMethod();

        // Get information on the parameter that is null so we can add its name to the exception
        ParameterInfo param = info.GetParameters()[index];

        return param;
    }
EJoshuaS - Przywróć Monikę
źródło
1
Dlaczego jest to niedoceniane? Nie jest tak źle
Gaspa79,
0

Nie sugeruję zgłaszania wyjątków od konstruktora. Problem polega na tym, że będziesz miał trudności z przetestowaniem tego, ponieważ musisz przekazać prawidłowe parametry, nawet jeśli nie mają one znaczenia dla twojego testu.

Na przykład: jeśli chcesz sprawdzić, czy trzeci parametr zgłasza wyjątek, musisz poprawnie podać pierwszy i drugi. Więc twój test nie jest już izolowany. gdy ograniczenia pierwszego i drugiego parametru zmienią się, ten przypadek testowy zawiedzie, nawet testuje trzeci parametr.

Proponuję użyć interfejsu API sprawdzania poprawności Java i zlecić proces sprawdzania poprawności na zewnątrz. Sugeruję zaangażowanie czterech obowiązków (zajęć):

  1. Obiekt sugestii z parametrami sprawdzania poprawności języka Java opatrzonymi adnotacjami parametrami za pomocą metody sprawdzania poprawności, która zwraca zestaw ograniczeń ograniczeń. Jedną z zalet jest to, że można ominąć te obiekty bez potwierdzenia ważności. Możesz opóźnić sprawdzanie poprawności, dopóki nie będzie to konieczne, bez konieczności tworzenia instancji obiektu domeny. Obiekt sprawdzania poprawności może być używany na różnych warstwach, ponieważ jest to POJO bez wiedzy specyficznej dla poszczególnych warstw. Może być częścią publicznego API.

  2. Fabryka obiektu domeny odpowiedzialna za tworzenie poprawnych obiektów. Przekazujesz obiekt sugestii, a fabryka wywoła sprawdzanie poprawności i utworzy obiekt domeny, jeśli wszystko będzie w porządku.

  3. Sam obiekt domeny, który powinien być w większości niedostępny dla instancji innych programistów. Do celów testowych sugeruję, aby mieć tę klasę w zasięgu pakietu.

  4. Interfejs domeny publicznej do ukrywania konkretnego obiektu domeny i utrudniania niewłaściwego użycia.

Nie sugeruję sprawdzania wartości pustych. Gdy tylko przejdziesz do warstwy domeny, pozbędziesz się przekazywania wartości null lub zwracania wartości null, o ile nie masz łańcuchów obiektów, które mają zakończenia lub funkcje wyszukiwania pojedynczych obiektów.

Jeszcze jedna kwestia: pisanie mniejszej ilości kodu nie jest miarą jakości kodu. Pomyśl o konkursach na 32 000 gier. Ten kod jest najbardziej kompaktowy, ale także najbardziej bałaganiarski, jaki możesz mieć, ponieważ dba on tylko o kwestie techniczne i nie dba o semantykę. Ale semantyka to punkty, które czynią wszystko kompleksowym.

oopexpert
źródło
1
Z szacunkiem nie zgadzam się z twoim ostatnim punktem. Nie trzeba wpisywać ten sam boilerplate po raz 100. ma pomóc zmniejszyć ryzyko popełnienia błędu. Wyobraź sobie, że to była Java, a nie C #. Musiałbym zdefiniować pole zaplecza i metodę gettera dla każdej właściwości. Kod stałby się całkowicie nieczytelny, a co ważne, byłaby zasłonięta przez niezgrabną infrastrukturę.
Botond Balázs