Sprawdzanie poprawności parametru wejściowego w wywołującym: duplikacja kodu?

16

Gdzie jest najlepsze miejsce do sprawdzania poprawności parametrów wejściowych funkcji: w funkcji wywołującej lub w samej funkcji?

Chciałbym ulepszyć swój styl kodowania, dlatego staram się znaleźć najlepsze praktyki lub reguły dotyczące tego problemu. Kiedy i co jest lepsze.

W moich poprzednich projektach sprawdzaliśmy i traktowaliśmy każdy parametr wejściowy wewnątrz funkcji (na przykład, jeśli nie jest on zerowy). Teraz przeczytałem tutaj w niektórych odpowiedziach, a także w książce Pragmatic Programmer, że sprawdzenie poprawności parametru wejściowego jest obowiązkiem osoby dzwoniącej.

Oznacza to, że powinienem sprawdzić parametry wejściowe przed wywołaniem funkcji. Wszędzie wywoływana jest funkcja. I to rodzi jedno pytanie: czy nie powoduje to powielania warunku sprawdzania wszędzie, gdzie wywoływana jest funkcja?

Nie interesują mnie tylko warunki zerowe, ale sprawdzanie poprawności dowolnych zmiennych wejściowych (ujemna wartość do sqrtfunkcji, dzielenie przez zero, niewłaściwa kombinacja stanu i kodu pocztowego lub cokolwiek innego)

Czy istnieją jakieś zasady, jak zdecydować, gdzie sprawdzić warunek wejściowy?

Myślę o kilku argumentach:

  • gdy traktowanie nieprawidłowej zmiennej może się różnić, dobrze jest sprawdzić ją po stronie wywołującej (np. sqrt()funkcja - w niektórych przypadkach mogę chcieć pracować ze złożoną liczbą, więc traktuję ten stan w wywołującym)
  • gdy warunek sprawdzania jest taki sam u każdego dzwoniącego, lepiej sprawdzić go w funkcji, aby uniknąć powielania
  • sprawdzanie poprawności parametru wejściowego w programie wywołującym odbywa się tylko jeden przed wywołaniem wielu funkcji z tym parametrem Dlatego sprawdzanie poprawności parametru w każdej funkcji nie jest skuteczne
  • właściwe rozwiązanie zależy od konkretnego przypadku

Mam nadzieję, że to pytanie nie jest duplikatem żadnego innego, szukałem tego problemu i znalazłem podobne pytania, ale nie wspominają dokładnie o tym przypadku.

srnka
źródło

Odpowiedzi:

15

To zależy. Decyzja o tym, gdzie umieścić walidację, powinna opierać się na opisie i sile umowy wynikającej z metody (lub udokumentowanej) . Walidacja jest dobrym sposobem na zwiększenie przestrzegania określonej umowy. Jeśli z jakiegokolwiek powodu metoda ma bardzo ścisłą umowę, to tak, to do ciebie należy sprawdzenie przed wywołaniem.

Jest to szczególnie ważna koncepcja podczas tworzenia metody publicznej , ponieważ zasadniczo reklamujesz, że niektóre metody wykonują pewne operacje. Lepiej rób to, co mówisz!

Weź jako przykład następującą metodę:

public void DeletePerson(Person p)
{            
    _database.Delete(p);
}

Z czego wynika umowa DeletePerson? Programista może jedynie założyć, że jeśli coś Personzostanie przekazane, zostanie usunięte. Wiemy jednak, że nie zawsze jest to prawdą. Co jeśli pjest nullwartością? Co jeśli pnie istnieje w bazie danych? Co się stanie, jeśli baza danych zostanie odłączona? Dlatego wydaje się , że DeletePerson nie wypełnia dobrze swojej umowy. Czasami usuwa osobę, a czasem zgłasza wyjątek NullReferenceException lub DatabaseNotConnectedException, a czasem nic nie robi (na przykład jeśli osoba została już usunięta).

Tego typu interfejsy API są niezwykle trudne w użyciu, ponieważ kiedy nazwiemy to „czarną skrzynką” metody, mogą się zdarzyć różnego rodzaju okropne rzeczy.

Oto kilka sposobów na ulepszenie umowy:

  • Dodaj weryfikację i dodaj wyjątek do umowy. To sprawia, że ​​umowa jest silniejsza , ale wymaga od osoby dzwoniącej sprawdzenia poprawności. Różnica polega jednak na tym, że teraz znają swoje wymagania. W tym przypadku komunikuję to z komentarzem w języku C # XML, ale zamiast tego możesz dodać throws(Java), użyć Assertlub użyć narzędzia kontraktowego, takiego jak Code Contracts.

    ///<exception>ArgumentNullException</exception>
    ///<exception>ArgumentException</exception>
    public void DeletePerson(Person p)
    {            
        if(p == null)
            throw new ArgumentNullException("p");
        if(!_database.Contains(p))
            throw new ArgumentException("The Person specified is not in the database.");
    
        _database.Delete(p);
    }
    

    Uwaga dodatkowa: Argumentem przeciwko temu stylowi jest często to, że powoduje ono nadmierną wstępną weryfikację przez cały kod wywołujący, ale z mojego doświadczenia często tak nie jest. Pomyśl o scenariuszu, w którym próbujesz usunąć pustą osobę. Jak to się stało? Skąd się wzięła Osoba pusta? Jeśli jest to na przykład interfejs użytkownika, dlaczego obsługiwano klawisz Delete, jeśli nie ma bieżącego wyboru? Jeśli został już usunięty, czy nie powinien być już usunięty z wyświetlacza? Oczywiście są wyjątki, ale w miarę rozwoju projektu często dziękujesz takiemu kodowi za zapobieganie przenikaniu błędów w głąb systemu.

  • Dodaj obronnie sprawdzanie poprawności i kod. To sprawia, że ​​umowa jest luźniejsza , ponieważ teraz ta metoda nie tylko usuwa osobę. Zmieniłem nazwę metody, aby to odzwierciedlić, ale może nie być konieczne, jeśli masz spójny interfejs API. To podejście ma swoje zalety i wady. Zaletą jest to, że możesz teraz wywoływać TryDeletePersonprzekazywanie przy użyciu różnego rodzaju nieprawidłowych danych wejściowych i nigdy nie martw się o wyjątki. Wadą jest oczywiście to, że użytkownicy Twojego kodu prawdopodobnie wywołują tę metodę zbyt często lub może to utrudnić debugowanie w przypadkach, gdy p jest zerowe. Można to uznać za łagodne naruszenie zasady pojedynczej odpowiedzialności , więc miej to na uwadze, jeśli wybuchnie wojna z płomieniem.

    public void TryDeletePerson(Person p)
    {            
        if(p == null || !_database.Contains(p))
            return;
    
        _database.Delete(p);
    }
    
  • Połącz podejścia. Czasami chcesz trochę z obu, gdy chcesz, aby zewnętrzne osoby dzwoniące ściśle przestrzegały zasad (aby zmusić ich do kodowania odpowiedzialnego), ale chcesz, aby Twój prywatny kod był elastyczny.

    ///<exception>ArgumentNullException</exception>
    ///<exception>ArgumentException</exception>
    public void DeletePerson(Person p)
    {            
        if(p == null)
            throw new ArgumentNullException("p");
        if(!_database.Contains(p))
            throw new ArgumentException("The Person specified is not in the database.");
    
        TryDeletePerson(p);
    }
    
    internal void TryDeletePerson(Person p)
    {            
        if(p == null || !_database.Contains(p))
            return;
    
        _database.Delete(p);
    }
    

Z mojego doświadczenia wynika, że ​​najlepiej koncentruje się na umowach, które sugerowałeś, a nie na sztywnych zasadach. Kodowanie obronne wydaje się działać lepiej w przypadkach, gdy osoba dzwoniąca ma trudności z ustaleniem, czy operacja jest poprawna. Ścisłe umowy wydają się działać lepiej tam, gdzie oczekujesz, że dzwoniący będzie wywoływał metody tylko wtedy, gdy naprawdę, naprawdę mają sens.

Kevin McCormick
źródło
Dzięki za bardzo miłą odpowiedź z przykładem. Podoba mi się podejście do „obrony” i „ścisłego kontraktu”.
srnka
7

Jest to kwestia konwencji, dokumentacji i przypadku użycia.

Nie wszystkie funkcje są równe. Nie wszystkie wymagania są równe. Nie wszystkie walidacje są równe.

Na przykład, jeśli twój projekt Java próbuje unikać wskaźników zerowych, gdy tylko jest to możliwe (zobacz na przykład zalecenia stylu Guava ), czy nadal sprawdzasz poprawność każdego argumentu funkcji, aby upewnić się, że nie jest on pusty? Prawdopodobnie nie jest to konieczne, ale istnieje prawdopodobieństwo, że nadal to robisz, aby ułatwić znajdowanie błędów. Ale możesz użyć twierdzenia, w którym wcześniej zgłosiłeś wyjątek NullPointerException.

Co jeśli projekt jest w C ++? Konwencja / tradycja w C ++ polega na dokumentowaniu warunków wstępnych, ale weryfikuj je (jeśli w ogóle) tylko w kompilacjach debugowania.

W obu przypadkach masz udokumentowane warunki wstępne dla swojej funkcji: żaden argument nie może być pusty. Zamiast tego możesz rozszerzyć domenę funkcji o wartości null ze zdefiniowanym zachowaniem, np. „Jeśli dowolny argument ma wartość null, zgłasza wyjątek”. Oczywiście, to znowu moje dziedzictwo C ++ mówi tutaj - w Javie jest wystarczająco powszechne, aby w ten sposób udokumentować warunki wstępne.

Ale nie wszystkie warunki wstępne można nawet rozsądnie sprawdzić. Na przykład algorytm wyszukiwania binarnego ma warunek, że sekwencja do przeszukania musi zostać posortowana. Ale sprawdzenie, czy tak jest na pewno, jest operacją O (N), więc robienie tego przy każdym wywołaniu niweczy konieczność użycia algorytmu O (log (N)). Jeśli programujesz defensywnie, możesz wykonać mniejszą kontrolę (np. Sprawdzenie, czy dla każdej wyszukiwanej partycji sortowane są wartości początkowa, środkowa i końcowa), ale to nie wychwytuje wszystkich błędów. Zazwyczaj będziesz musiał polegać na spełnieniu warunku wstępnego.

Jedynym prawdziwym miejscem, w którym potrzebujesz jawnych kontroli, są granice. Zewnętrzny wkład do twojego projektu? Sprawdź poprawność, sprawdź poprawność, sprawdź poprawność. Szary obszar to granice API. To naprawdę zależy od tego, jak bardzo chcesz zaufać kodowi klienta, ile szkód wyrządza nieprawidłowe wejście i jak dużo pomocy chcesz zapewnić w poszukiwaniu błędów. Wszelkie granice uprawnień muszą się oczywiście liczyć jako zewnętrzne - na przykład syscalls, uruchamiane w kontekście podwyższonych uprawnień i dlatego należy bardzo uważać, aby sprawdzić poprawność. Każda taka walidacja musi oczywiście dotyczyć wewnętrznego połączenia systemowego.

Sebastian Redl
źródło
Dziękuję za odpowiedź. Czy możesz podać link do rekomendacji stylu Guava? Nie mogę google i dowiedzieć się, co masz na myśli. +1 do walidacji granic.
srnka
Dodano link. W rzeczywistości nie jest to pełny przewodnik po stylu, tylko część dokumentacji narzędzi innych niż zero.
Sebastian Redl,
6

Sprawdzanie poprawności parametrów powinno być przedmiotem wywoływanej funkcji. Funkcja powinna wiedzieć, co jest uważane za prawidłowe wejście, a co nie. Dzwoniący mogą tego nie wiedzieć, zwłaszcza gdy nie wiedzą, jak funkcja jest zaimplementowana wewnętrznie. Należy oczekiwać, że funkcja będzie obsługiwać dowolną kombinację wartości parametrów wywołujących.

Ponieważ funkcja jest odpowiedzialna za sprawdzanie poprawności parametrów, możesz napisać testy jednostkowe dla tej funkcji, aby upewnić się, że zachowuje się ona zgodnie z przeznaczeniem zarówno z poprawnymi, jak i nieprawidłowymi wartościami parametrów.

Bernard
źródło
Dziękuję za odpowiedź. Więc uważasz, że ta funkcja powinna sprawdzać zarówno prawidłowe, jak i nieprawidłowe parametry wejściowe w każdym przypadku. Coś innego niż potwierdzenie książki Pragmatic Programmer: „sprawdzenie parametru wejściowego jest obowiązkiem osoby dzwoniącej”. Miło jest pomyśleć „Funkcja powinna wiedzieć, co jest uważane za prawidłowe… Dzwoniący mogą tego nie wiedzieć”… Więc nie lubisz korzystać z warunków wstępnych?
srnka
1
Możesz użyć warunków wstępnych, jeśli chcesz (patrz odpowiedź Sebastiana ), ale wolę być defensywny i obsługiwać wszelkie możliwe dane wejściowe.
Bernard
4

W samej funkcji. Jeśli funkcja jest używana więcej niż raz, nie chcesz weryfikować parametru dla każdego wywołania funkcji.

Ponadto, jeśli funkcja zostanie zaktualizowana w taki sposób, że wpłynie to na sprawdzanie poprawności parametru, musisz wyszukać każde wystąpienie sprawdzania poprawności dzwoniącego, aby je zaktualizować. To nie jest piękne :-).

Możesz odnieść się do klauzuli ochronnej

Aktualizacja

Zobacz moją odpowiedź dla każdego przedstawionego scenariusza.

  • gdy traktowanie nieprawidłowej zmiennej może się różnić, dobrze jest sprawdzić ją po stronie wywołującej (np. sqrt()funkcja - w niektórych przypadkach mogę chcieć pracować ze złożoną liczbą, więc traktuję ten stan w wywołującym)

    Odpowiedź

    Większość języków programowania domyślnie obsługuje liczby całkowite i rzeczywiste, a nie liczby zespolone, dlatego ich implementacja sqrtprzyjmuje tylko liczbę nieujemną. Jedynym przypadkiem, w którym masz sqrtfunkcję, która zwraca liczbę zespoloną, jest użycie języka programowania zorientowanego na matematykę, takiego jak Mathematica

    Co więcej, sqrtdla większości języków programowania jest już zaimplementowany, dlatego nie można go zmodyfikować, a jeśli spróbujesz zastąpić implementację (patrz łatanie małp), wówczas Twoi współpracownicy będą całkowicie szokujący, dlaczego sqrtnagle akceptuje liczby ujemne.

    Jeśli chcesz, możesz owinąć go wokół swojej sqrtfunkcji niestandardowej, która obsługuje liczbę ujemną i zwraca liczbę zespoloną.

  • gdy warunek sprawdzania jest taki sam u każdego dzwoniącego, lepiej sprawdzić go w funkcji, aby uniknąć powielania

    Odpowiedź

    Tak, jest to dobra praktyka, aby uniknąć rozproszenia sprawdzania poprawności parametrów w całym kodzie.

  • sprawdzanie poprawności parametru wejściowego w programie wywołującym odbywa się tylko jeden przed wywołaniem wielu funkcji z tym parametrem Dlatego sprawdzanie poprawności parametru w każdej funkcji nie jest skuteczne

    Odpowiedź

    Fajnie będzie, jeśli dzwoniący jest funkcją, nie sądzisz?

    Jeśli funkcje w ramach wywołującego są używane przez innego wywołującego, co uniemożliwia sprawdzenie poprawności parametru w funkcjach wywoływanych przez wywołującego?

  • właściwe rozwiązanie zależy od konkretnego przypadku

    Odpowiedź

    Cel kodu, który można utrzymać. Przeniesienie sprawdzania poprawności parametrów zapewnia jedno źródło prawdy na temat tego, co funkcja może zaakceptować lub nie.

Onesimus Bez ograniczeń
źródło
Dziękuję za odpowiedź. Funkcja sqrt () była tylko przykładem, to samo zachowanie z parametrem wejściowym może być wykorzystane przez wiele innych funkcji. „jeśli funkcja zostanie zaktualizowana w taki sposób, że wpłynie to na sprawdzenie poprawności parametru, musisz wyszukać każde wystąpienie sprawdzania poprawności dzwoniącego” - nie zgadzam się z tym. Możemy wtedy powiedzieć to samo o wartości zwracanej: jeśli funkcja zostanie zaktualizowana w taki sposób, że wpłynie na wartość zwracaną, musisz poprawić każdego dzwoniącego ... Myślę, że funkcja musi mieć jedno dobrze zdefiniowane zadanie do wykonania ... W przeciwnym razie i tak zmiana dzwoniącego jest konieczna.
srnka
2

Funkcja powinna określać swoje warunki wstępne i wstępne.
Warunki wstępne to warunki, które musi spełnić osoba dzwoniąca, zanim będzie mogła poprawnie korzystać z funkcji i może (i często tak) obejmować ważność parametrów wejściowych.
Warunki dodatkowe są obietnicami, które funkcja składa swoim dzwoniącym.

Jeżeli ważność parametrów funkcji jest częścią warunków wstępnych, to na dzwoniącym spoczywa obowiązek upewnienia się, że parametry te są prawidłowe. Ale to nie znaczy, że każdy dzwoniący musi wyraźnie sprawdzić każdy parametr przed połączeniem. W większości przypadków nie są wymagane żadne jawne testy, ponieważ wewnętrzna logika i warunki wstępne programu wywołującego już zapewniają, że parametry są prawidłowe.

Aby zabezpieczyć się przed błędami programistycznymi (błędami), można sprawdzić, czy parametry przekazane do funkcji rzeczywiście spełniają podane warunki wstępne. Ponieważ testy te mogą być kosztowne, dobrym pomysłem jest ich wyłączenie w kompilacjach wersji. Jeśli te testy zakończą się niepowodzeniem, program powinien zostać zakończony, ponieważ prawdopodobnie wystąpił błąd.

Chociaż na pierwszy rzut oka sprawdzanie w programie wywołującym wydaje się zachęcać do powielania kodu, w rzeczywistości jest odwrotnie. Kontrola w odbierającym powoduje duplikację kodu i wiele niepotrzebnych prac.
Pomyśl o tym, jak często przekazujesz parametry przez kilka warstw funkcji, dokonując tylko niewielkich zmian w niektórych z nich po drodze. Jeśli konsekwentnie stosujesz metodę sprawdzania w callee , każda z tych funkcji pośrednich będzie musiała ponownie wykonać kontrolę dla każdego z parametrów.
A teraz wyobraź sobie, że jednym z tych parametrów powinna być posortowana lista.
Po sprawdzeniu dzwoniącego tylko pierwsza funkcja musiałaby upewnić się, że lista jest naprawdę posortowana. Wszyscy inni wiedzą, że lista jest już posortowana (ponieważ tak stwierdzili we wstępnym stanie) i mogą ją przekazywać bez dalszych kontroli.

Bart van Ingen Schenau
źródło
+1 dzięki za odpowiedź. Niezła refleksja: „Kontrola w odbierającym powoduje duplikację kodu i wiele niepotrzebnych prac”. I w zdaniu: „W większości przypadków nie są wymagane żadne wyraźne testy, ponieważ logika wewnętrzna i warunki wstępne osoby dzwoniącej już zapewniają” - co masz na myśli przez wyrażenie „logika wewnętrzna”? Funkcjonalność DBC?
srnka
@ srnka: Przez „logikę wewnętrzną” mam na myśli obliczenia i decyzje w funkcji. Zasadniczo jest to implementacja funkcji.
Bart van Ingen Schenau
0

Najczęściej nie wiesz, kto, kiedy i jak wywoła napisaną funkcję. Najlepiej założyć najgorsze: twoja funkcja zostanie wywołana z niepoprawnymi parametrami. Więc zdecydowanie powinieneś to zakryć.

Niemniej jednak, jeśli używany język obsługuje wyjątki, możesz nie sprawdzić pewnych błędów i upewnić się, że wyjątek zostanie zgłoszony, ale w takim przypadku musisz koniecznie opisać sprawę w dokumentacji (musisz mieć dokumentację). Wyjątek dostarczy rozmówcy wystarczającą ilość informacji o tym, co się wydarzyło, a także zwróci uwagę na nieprawidłowe argumenty.

superM
źródło
W rzeczywistości może być lepiej sprawdzić poprawność parametru, a jeśli parametr jest nieprawidłowy, sam rzuć wyjątek. Oto dlaczego: klauni, którzy dzwonią do twojej procedury bez zawracania sobie głowy upewniając się, że podali jej prawidłowe dane, to ci sami, którzy nie zawracają sobie głowy sprawdzaniem kodu powrotu błędu, który wskazuje, że przekazali nieprawidłowe dane. Zgłoszenie wyjątku WYMUSZA problem, który należy naprawić.
John R. Strohm,