Czy musimy zweryfikować użycie całego modułu, czy tylko argumenty metod publicznych?

9

Słyszałem, że zalecane jest sprawdzenie poprawności argumentów metod publicznych:

Motywacja jest zrozumiała. Jeśli moduł zostanie użyty w niewłaściwy sposób, chcemy natychmiast rzucić wyjątek zamiast nieprzewidzianego zachowania.

Niepokoi mnie to, że błędne argumenty nie są jedynym błędem, który można popełnić podczas korzystania z modułu. Oto kilka scenariuszy błędów, w których musimy dodać logikę sprawdzania, jeśli postępujemy zgodnie z zaleceniami i nie chcemy eskalacji błędów:

  • Połączenie przychodzące - nieoczekiwane argumenty
  • Połączenie przychodzące - moduł jest w złym stanie
  • Połączenie zewnętrzne - zwrócono nieoczekiwane wyniki
  • Połączenie zewnętrzne - nieoczekiwane skutki uboczne (podwójne wejście do modułu wywołującego, przełamanie innych stanów zależności)

Próbowałem wziąć pod uwagę wszystkie te warunki i napisać prosty moduł z jedną metodą (przepraszam, nie-C # faceci):

public sealed class Room
{
    private readonly IDoorFactory _doorFactory;
    private bool _entered;
    private IDoor _door;
    public Room(IDoorFactory doorFactory)
    {
        if (doorFactory == null)
            throw new ArgumentNullException("doorFactory");
        _doorFactory = doorFactory;
    }
    public void Open()
    {
        if (_door != null)
            throw new InvalidOperationException("Room is already opened");
        if (_entered)
            throw new InvalidOperationException("Double entry is not allowed");
        _entered = true;
        _door = _doorFactory.Create();
        if (_door == null)
            throw new IncompatibleDependencyException("doorFactory");
        _door.Open();
        _entered = false;
    }
}

Teraz jest bezpiecznie =)

To dość przerażające. Ale wyobraź sobie, jak przerażający może być w prawdziwym module z dziesiątkami metod, złożonym stanem i wieloma zewnętrznymi wywołaniami (cześć, miłośnicy wstrzykiwania zależności!). Zauważ, że jeśli wywołujesz moduł, którego zachowanie może zostać zastąpione (klasa nie zapieczętowana w języku C #), wówczas wykonujesz wywołanie zewnętrzne, a konsekwencje nie są przewidywalne w zakresie wywołującego.

Podsumowując, jaki jest właściwy sposób i dlaczego? Jeśli możesz wybrać jedną z poniższych opcji, odpowiedz na dodatkowe pytania.

Sprawdź użycie całego modułu. Czy potrzebujemy testów jednostkowych? Czy są przykłady takiego kodu? Czy zastrzyk zależności powinien być ograniczony w użyciu (ponieważ spowoduje to więcej logiki sprawdzania)? Czy nie jest praktyczne przeniesienie tych czeków na czas debugowania (nie są uwzględniane w wersji)?

Sprawdź tylko argumenty. Z mojego doświadczenia wynika, że ​​sprawdzanie argumentów - szczególnie sprawdzanie wartości zerowej - jest najmniej skutecznym sprawdzaniem, ponieważ błąd argumentu rzadko prowadzi do skomplikowanych błędów i eskalacji błędów. Przez większość czasu dostaniesz NullReferenceExceptionw następnym wierszu. Dlaczego więc sprawdzanie argumentów jest tak szczególne?

Nie sprawdzaj użycia modułu. To dość niepopularna opinia, czy możesz wyjaśnić, dlaczego?

astef
źródło
Kontrole powinny być przeprowadzane podczas przydzielania pola, aby zapewnić zachowanie niezmienników.
Basilevs,
@Basilevs Interesujące ... Czy to z ideologii Code Contracts czy z czegoś starszego? Czy możesz polecić coś do przeczytania (związane z Twoim komentarzem)?
od
To podstawowy podział problemów. Wszystkie twoje przypadki są objęte gwarancją, podczas gdy powielanie kodu jest minimalne, a zakres obowiązków jest dobrze określony.
Basilevs,
@Basilevs Więc nie sprawdzaj w ogóle zachowania innych modułów, ale sprawdź niezmienniki własnego stanu. Brzmi rozsądnie. Ale dlaczego nie widzę tego prostego potwierdzenia w powiązanych pytaniach dotyczących sprawdzania argumentów?
dalej
Cóż, niektóre kontrole są nadal potrzebne, ale powinny być wykonywane tylko na rzeczywiście używanych wartościach, a nie na tych, które są przekazywane w innym miejscu. Na przykład polegasz na implementacji listy, aby sprawdzić błędy OOB, w przeciwieństwie do sprawdzania indeksu w kodzie klienta. Zwykle są to awarie szkieletowe niskiego poziomu i nie wymagają ręcznej emisji.
Basilevs,

Odpowiedzi:

2

TL; DR: Zatwierdź zmianę stanu, polegaj na [aktualności] bieżącego stanu.

Poniżej rozważam tylko weryfikacje z włączoną wersją. Aktywne asercje tylko do debugowania są formą dokumentacji, która jest przydatna na swój sposób i nie mieści się w zakresie tego pytania.

Rozważ następujące zasady:

  • Zdrowy rozsądek
  • Szybko zawieść
  • SUCHY
  • SRP

Definicje

  • Component - jednostka zapewniająca API
  • Klient - użytkownik interfejsu API komponentu

Zmienny stan

Problem

W językach imperatywnych objaw błędu i jego przyczyna mogą być oddzielone godzinami ciężkiego podnoszenia. Korupcja państwa może się ukrywać i mutować, powodując niewytłumaczalną awarię, ponieważ kontrola obecnego stanu nie może ujawnić pełnego procesu korupcji, a zatem źródła błędu.

Rozwiązanie

Każda zmiana stanu powinna być starannie opracowana i zweryfikowana. Jednym ze sposobów radzenia sobie ze stanem zmiennym jest ograniczenie go do minimum. Osiąga się to poprzez:

  • system typów (deklaracje stałych i ostatecznych członków)
  • wprowadzanie niezmienników
  • weryfikowanie każdej zmiany stanu komponentu za pomocą publicznych interfejsów API

Rozbudowując stan komponentu, rozważ to, pozwalając kompilatorowi na wymuszenie niezmienności nowych danych. Wymuszaj także każde rozsądne ograniczenie środowiska wykonawczego, ograniczając potencjalne stany wynikowe do najmniejszego możliwego dobrze zdefiniowanego zestawu.

Przykład

// Wrong
class Natural {
    private int number;
    public Natural(int number) {
        this.number = number;
    }
    public int getInt() {
      if (number < 1)
          throw new InvalidOperationException();
      return number;
    }
}

// Right
class Natural {
    private readonly int number;
    /**
     * @param number - positive number
     */
    public Natural(int number) {
      // Going to modify state, verification is required
      if (number < 1)
        throw new ArgumentException("Natural number should be  positive: " + number);
      this.number = number;
    }
    public int getInt() {
      // State is guaranteed by construction and compiler
      return number;
    }
}

Powtarzalność i spójność odpowiedzialności

Problem

Sprawdzanie warunków wstępnych i końcowych warunków operacji prowadzi do duplikacji kodu weryfikacyjnego zarówno w kliencie, jak i komponencie. Sprawdzanie poprawności wywołania komponentu często zmusza klienta do przejęcia niektórych obowiązków komponentu.

Rozwiązanie

Polegaj na komponencie, aby w miarę możliwości przeprowadzić weryfikację stanu. Komponenty mają zapewniać interfejs API, który nie wymaga specjalnej weryfikacji użytkowania (na przykład weryfikacji argumentów lub egzekwowania sekwencji operacji), aby utrzymać stan komponentu, który jest dobrze zdefiniowany. Zobowiązują się do weryfikacji argumentów wywołania API zgodnie z wymaganiami, zgłaszania awarii za pomocą niezbędnych środków i dążą do zapobiegania ich korupcji.

Klienci powinni polegać na komponentach w celu weryfikacji korzystania z ich API. Unika się nie tylko powtarzania, klient nie zależy już od dodatkowych szczegółów implementacyjnych komponentu. Rozważ szkielet jako komponent. Napisz niestandardowy kod weryfikacyjny tylko wtedy, gdy niezmienniki komponentu nie są wystarczająco rygorystyczne lub aby zawrzeć wyjątek komponentu jako szczegół implementacji.

Jeśli operacja nie zmienia stanu i nie jest objęta weryfikacją zmiany stanu, sprawdź każdy argument na najgłębszym możliwym poziomie.

Przykład

class Store {
  private readonly List<int> slots = new List<int>();
  public void putToSlot(int slot, int data) {
    if (slot < 0 || slot >= slots.Count) // Unnecessary, validated by List, only needed for custom error message
      throw new ArgumentException("data");
    slots[slot] = data;
  }
}

class Natural {
   int _number;
   public Natural(int number) {
       if (number < 1)
          number = 1;  //Wrong: client can't rely on argument verification, additional state uncertainity is introduced.  Right: throw new ArgumentException(number);
       _number = number;
   }
}

Odpowiedź

Gdy opisane zasady zostaną zastosowane w danym przykładzie, otrzymamy:

public sealed class Room
{
    private bool _entered = false;
    // Do not use lazy instantiation if not absolutely necessary, this introduces additional mutable state
    private readonly IDoor _door;
    public Room(IDoorFactory doorFactory)
    {
        // Rely on system null check
        IDoor door = _doorFactory.Create();
        // Modifying own state, verification is required
        if (door == null)
           throw new ArgumentNullException("Door");
        _door = door;
    }
    public void Enter()
    {
        // Room invariants do not guarantee _entered value. Door state is indirectly a part of our state. Verification is required to prevent second door state change below.
        if (_entered)
           throw new InvalidOperationException("Double entry is not allowed");
        _entered = true;     
        // rely on immutability for _door field to be non-null
        // rely on door implementation to control resulting door state       
        _door.Open();            
    }
}

Podsumowanie

Stan klienta składa się z wartości własnych pól i części stanu składnika, które nie są objęte jego niezmiennikami. Weryfikacji należy dokonać tylko przed faktyczną zmianą stanu klienta.

Basilevs
źródło
1

Klasa jest odpowiedzialna za swój własny stan. Zweryfikuj więc w zakresie, w jakim utrzymuje lub wprowadza rzeczy do akceptowalnego stanu.

Jeśli moduł zostanie użyty w niewłaściwy sposób, chcemy natychmiast rzucić wyjątek zamiast nieprzewidzianego zachowania.

Nie, nie rzucaj wyjątku, zamiast tego zapewnij przewidywalne zachowanie. Następstwem odpowiedzialności państwa jest uczynienie klasy / aplikacji tak tolerancyjnym, jak praktycznym. Na przykład przechodząc nulldo aCollection.Add()? Tylko nie dodawaj i kontynuuj. Dostajesz nulldane wejściowe do utworzenia obiektu? Utwórz obiekt zerowy lub domyślny. Powyżej doorjest już open? Więc co dalej? DoorFactoryargument jest zerowy? Utwórz nowy. Kiedy tworzę enum, zawsze mam Undefinedczłonka. Korzystam liberalnie z Dictionarys i enumsjasno definiuję rzeczy; a to prowadzi długą drogę do zapewnienia przewidywalnego zachowania.

(Cześć, miłośnicy wstrzykiwania zależności!)

Tak, choć idę przez cień doliny parametrów, nie będę się obawiać żadnych argumentów. Do powyższego używam również parametrów domyślnych i opcjonalnych w jak największym stopniu.

Wszystkie powyższe pozwalają na kontynuację wewnętrznego przetwarzania. W konkretnej aplikacji mam dziesiątki metod w wielu klasach z jednym miejscem, w którym zgłaszany jest wyjątek. Nawet wtedy nie dzieje się tak z powodu argumentów zerowych lub tego, że nie mogłem kontynuować przetwarzania, to dlatego, że kod ostatecznie stworzył obiekt „niefunkcjonalny” / „zerowy”.

edytować

cytując mój komentarz w całości. Myślę, że projekt nie powinien po prostu „poddawać się”, gdy pojawia się „zero”. Zwłaszcza przy użyciu obiektu złożonego.

Zapominamy tutaj o kluczowych koncepcjach / założeniach - encapsulationi single responsibility. Po pierwszej warstwie interakcji z klientem praktycznie nie ma sprawdzania wartości NULL. Kod jest odporny na uszkodzenia . Klasy są projektowane z domyślnymi stanami, więc działają bez pisania, jakby interakcyjny kod był zainfekowany błędami, nieuczciwymi śmieciami. Złożony element nadrzędny nie musi sięgać w dół warstw podrzędnych, aby ocenić poprawność (i domyślnie sprawdzić, czy we wszystkich zakątkach nie ma wartości null). Rodzic wie, co oznacza domyślny stan dziecka

zakończ edycję

radarbob
źródło
1
Brak dodania nieprawidłowego elementu kolekcji jest bardzo nieprzewidywalnym zachowaniem.
Basilevs,
1
Jeśli wszystkie interfejsy zostaną zaprojektowane w taki tolerancyjny sposób, pewnego dnia z powodu banalnego błędu programy przypadkowo się obudzą i zniszczą ludzkość.
ok.
Zapominamy tutaj o kluczowych koncepcjach / założeniach - encapsulationi single responsibility. nullPo pierwszej warstwie interakcji z klientem praktycznie nie ma sprawdzania. Kod <strike> tolerancyjny </strike> jest niezawodny. Klasy są projektowane z domyślnymi stanami, więc działają bez pisania, jakby interakcyjny kod był zainfekowany błędami, nieuczciwymi śmieciami. Złożony element nadrzędny nie musi sięgać w dół warstw podrzędnych, aby ocenić poprawność (i domyślnie sprawdzić, czy nullwe wszystkich zakątkach). Rodzic wie, co oznacza domyślny stan dziecka
radarbob