Usługa powinna zgłosić wyjątek lub zwrócić, gdy nie określono żadnych elementów do usunięcia

12

Mam fragment kodu, który można przedstawić jako:

public class ItemService {

    public void DeleteItems(IEnumerable<Item> items)
    {
        // Save us from possible NullReferenceException below.
        if(items == null)
            return;

        foreach(var item in items)
        {
            // For the purpose of this example, lets say I have to iterate over them.
            // Go to database and delete them.
        }
    }
}

Teraz zastanawiam się, czy jest to właściwe podejście, czy powinienem rzucić wyjątek. Mogę uniknąć wyjątku, ponieważ zwracanie byłoby tym samym, co iteracja po pustej kolekcji, co oznacza, że ​​i tak żaden ważny kod nie jest wykonywany, ale z drugiej strony prawdopodobnie ukryję gdzieś problemy w kodzie, ponieważ dlaczego ktoś miałby dzwonić DeleteItemsz nullparametrem? Może to oznaczać, że w kodzie występuje problem.

Jest to problem, który zwykle mam z metodami w usługach, ponieważ większość z nich coś robi i nie zwraca wyniku, więc jeśli ktoś przekaże nieprawidłowe informacje, usługa nie ma nic do zrobienia, więc zwraca.

FCin
źródło
2
To tylko moja opinia, ale zawsze uważałem, że ma sens, aby metoda generowała wyjątek, gdy robi coś niezamierzonego (wyjątkowego). W twoim przypadku wyrzuciłbym InvalidOperationException, gdyby ktoś próbował usunąć elementy null / 0.
Falgantil
1
@gnat Moje pytanie dotyczy w szczególności usług, ponieważ są one wykorzystywane i ich przeznaczenie. Ten „duplikat” mówi tylko o ogólnych przypadkach, a główna odpowiedź jest nawet sprzeczna w kontekście mojej dokładnej metody.
FCin
2
Czy możliwe jest, aby wyliczenie było puste zamiast zera? Czy poradziłbyś sobie z tym inaczej?
JAD
13
@Falgantil Widzę, że null zasługuje na wyjątek, ale myślę, że absurdem jest umieszczenie wyjątku na pustej liście.
Kevin

Odpowiedzi:

58

To są dwa różne pytania.

Czy powinieneś zaakceptować null? To zależy od twojej ogólnej zasady nullw bazie kodu. Moim zdaniem banowanie nullwszędzie, z wyjątkiem przypadków, w których jest to wyraźnie udokumentowane, jest bardzo dobrą praktyką, ale jeszcze lepszą praktyką jest przestrzeganie konwencji, którą posiada już twoja baza kodu.

Czy powinieneś zaakceptować pustą kolekcję? Moim zdaniem: TAK, absolutnie. Znacznie większy wysiłek stanowi ograniczenie wszystkich osób dzwoniących do niepustych kolekcji niż zrobienie matematycznie właściwej rzeczy - nawet jeśli zaskakuje to niektórych programistów, którzy nie mają pojęcia o zero.

Kilian Foth
źródło
9
W każdym razie, jeśli masz zamiar rzucać, wtedy rzucanie nie ma aż tak dużej użyteczności, AhaINoticedYouPassingInNullExceptiongdy środowisko wykonawcze już zapewnia ci możliwość rzucania NullReferenceExceptionza ciebie, bez pisania żadnego kodu. Istnieje pewne narzędzie (np. Twoje narzędzie do pokrycia kodu powie ci, czy masz test jednostkowy na przekazanie wartości null w pierwszym przypadku, ale nie w drugim), ale żaden wyjątek nie powinien być próbowany / wychwytywany przy każdym wywołaniu usługi, ponieważ zwykle jest to błąd programisty.
Steve Jessop
2
... powinieneś natychmiast wychwytywać wyjątki zgłoszone z powodu szkicowych parametrów, jeśli wiesz, że to, co przekazujesz, jest w niektórych przypadkach spodziewane zero, i aktywnie chcesz, aby funkcja, którą wywołujesz, zweryfikowała ją dla Ciebie. Jak mówi Kilian w odpowiedzi, ogólną zasadą może być banowanie wartości null wszędzie, gdzie nie jest to wyraźnie dozwolone. Wtedy nie złapiesz NullReferenceExceptionani AhaINoticedYouPassingInNullExceptionnigdzie indziej, niż na wysokim poziomie kodu odzyskiwania po awarii. I ten program obsługi wyjątków powinien prawdopodobnie utworzyć raport o błędzie :-)
Steve Jessop
2
@FCin: Twój projekt interfejsu powinien być zgodny z tym, czego użytkownicy faktycznie potrzebują z usługi. Jeśli więc każdy dzwoniący zamierza zastosować metodę catch / catch, wówczas ta metoda lub inna metoda wygody obok tego powinna sprawdzić i zignorować wartość null, ponieważ tego właśnie wymaga społeczeństwo. Jednak, jak mówi Kilian, zwykle lepiej jest traktować propagowanie wartości zerowych jako błąd programowy i nie próbować kontynuować na każdej stronie wywołania. W tym przypadku, co jeśli usługa, na której wywoływana jest ta metoda, to null- czy kontrolery zawierają płytę kotłową do przechwycenia i kontynuowania również w tym przypadku?
Steve Jessop
2
... jeśli tak, to wygląda na to, że baza kodu konsekwentnie traktuje brakujące komponenty jako oczekiwany warunek, który powinieneś obsłużyć na niskim poziomie i kontynuować. Można racjonalnie zapytać: „na kim spoczywa obowiązek świadczenia usługi i wyliczenia, aby ta operacja mogła być kontynuowana?”. Jeśli odpowiedź brzmi „ktoś”, wówczas funkcja sprawdza warunki wstępne , a wynikiem niepowodzenia warunku normalnego byłby wyjątek wychwytywany na wysokim poziomie, a nie przy każdym wywołaniu. Jeśli wymagane elementy operacji są naprawdę opcjonalne, oznacza to, że funkcja obsługuje oczekiwany przypadek użycia .
Steve Jessop
2
Jawne rzucanie aniżeli ArgumentNullExceptionpozwala na wyrzucenie kodu wewnętrznego NullReferenceException- po prostu patrząc na komunikat wyjątku jest oczywiste, że jest to błąd wejściowy, a nie błąd logiczny w treści metody w miejscu rzucania, a zapewnienie wczesnego wyjścia oznacza, że bardziej prawdopodobne jest, że pozostawisz inne dane w poprawnym stanie, a nie częściowo zmutowane później z nieoczekiwanego null.
Miral
9

Wartość zerowa

Jak już powiedział @KilianFoth, trzymaj się swojej ogólnej polityki. Jeśli ma to być traktowane nulljako „skrót” dla pustej listy, zrób to w ten sposób.

Jeśli nie masz spójnych zasad dotyczących nullwartości, polecam następującą:

nullpowinny być zarezerwowane do reprezentowania sytuacji, których nie można wyrazić typem „normalnym”, np. używanie nulldo reprezentowania „Nie wiem”. I to dobry wybór, ponieważ każdy, kto spróbuje niedbale wykorzystać tę wartość, otrzyma wyjątek, co jest słuszne.

Użycie nullskrótu do pustej listy nie kwalifikuje się w ten sposób, ponieważ istnieje już idealna reprezentacja, będąc listą z zerowymi elementami. Jest to technicznie zły wybór, ponieważ zmusza każdą część kodu zajmującą się listami do sprawdzenia poprawności skrótu null.

Pusta lista

W przypadku DeleteItems()metody przekazanie pustej listy skutecznie oznacza nic nie robienie. Pozwoliłbym na to jako argument, nie rzucając wyjątku, po prostu wracając szybko.

Oczywiście, osoba dzwoniąca może najpierw sprawdzić zero elementów i DeleteItems()w takim przypadku pominąć połączenie. Jeśli mówimy o internetowym interfejsie API, ze względów wydajności osoba dzwoniąca powinna to zrobić, aby uniknąć niepotrzebnego ruchu i opóźnień w obie strony. Ale nie sądzę, że Twój interfejs API powinien to wymuszać.

Ralf Kleberhoff
źródło
1

Zgłaszaj wyjątek i obsługuj wartości zerowe w kodzie wywołującym.

Z reguły należy unikać wartości null jako wartości parametrów. Zmniejszy to ogólnie NullPointerExceptions, ponieważ wartości null będą naprawdę wyjątkiem.

Poza tym spójrz na resztę swojego kodu. Jeśli jest to powszechny wzór w twoim projekcie, zachowaj spójność.

serprime
źródło
Jestem w trakcie „kształtowania” struktury moich usług, więc może być konieczne trochę refaktoryzacji w celu wprowadzenia nieprawidłowych danych wejściowych, ale nie za dużo. Ale zgadzam się z twoją tezą, że wartości zerowe staną się wyjątkiem, gdy zacznę rzucać, zamiast je ukrywać.
FCin
0

Ogólnie rzecz biorąc, zgłaszanie wyjątków powinno być zarezerwowane na wyjątkowe sytuacje, tj. Jeśli kod nie ma rozsądnego sposobu działania w obecnym kontekście.

Możesz zastosować ten proces myślowy do tej sytuacji. Biorąc pod uwagę, że jest to klasa publiczna z publiczną metodą, masz publiczny interfejs API i teoretycznie nie masz kontroli nad tym, co zostanie mu przekazane.

Twój kod nie ma kontekstu na temat tego, co go nazywa (jak nie powinien), i nic nie możesz zrobić z wartością zerową. Z pewnością byłby to kandydat na ArgumentNullException.

Richzilla
źródło
„jeśli kod nie ma rozsądnego sposobu działania w obecnym kontekście”. Ale myślę, że ma rozsądny sposób działania, który ma powrócić do pustki. Robi dokładnie to samo, co przy przekazywaniu pustej kolekcji, więc jeśli pozwolę na pustą kolekcję, dlaczego nie pozwolę null.
FCin
1
@FCin Z powodu pustej kolekcji wiesz, że jest pusta. Z nulltobą nie masz kolekcji. Jeśli kod wywołujący ma / powinien dostarczyć kolekcję do metody, coś jest nie tak, więc powinieneś rzucić. Jedynym powodem traktowania wartości zerowej tak samo jak pustej kolekcji jest uzasadnione oczekiwanie, że kod wywołujący wygeneruje kolekcje zerowe. Jak zauważyły ​​inne odpowiedzi, w większości przypadków coś nulljest anomalią. Jeśli traktujesz je tak samo jak puste kolekcje, potencjalnie ignorujesz problem w innym miejscu kodu.
JAD
@FCin: również, jeśli rozumiesz nullpusty, jest to kontekstowe dla tej metody. Gdzieś indziej w tej samej bazie kodu, możesz mieć parametr IEnumerablelub IContainer, który dokonuje pewnego rodzaju filtrowania, nullmoże oznaczać „brak filtra” (zezwalaj na wszystko), podczas gdy pusty filtr oznacza, że ​​nic nie zezwala. A w innym miejscu nullmoże wyraźnie oznaczać „nie wiem”. Biorąc pod uwagę, że nullnie zawsze oznacza to wszędzie to samo, dobrym pomysłem jest, aby w ogóle nie wymyślać żadnego znaczenia, chyba że to znaczenie jest konieczne lub przynajmniej przydatne dla kogoś.
Steve Jessop
0

To pytanie wydaje się nie dotyczyć wyjątków, ale nullbyć ważnym argumentem.

Przede wszystkim musisz zdecydować, czy nullargument jest dozwolony dla argumentu tej metody. Jeśli tak, to nie potrzebujesz wyjątku. Jeśli tak nie jest, potrzebujesz wyjątku.

To, czy chcesz na to zezwolić null, czy nie, jest dyskusyjne, o czym świadczy wiele hitów Google na ten temat. Oznacza to, że nie dostaniesz jasnej odpowiedzi i zależy to w pewnym stopniu od opinii i tradycji w miejscu pracy.

Inną kwestią sporną jest to, czy funkcja biblioteczna powinna być naprawdę surowa w stosunku do takich rzeczy, czy tak łagodna, jak to możliwe, o ile nie próbuje ona „naprawić” błędnych parametrów. Porównaj to ze światem protokołów sieciowych, przesyłania poczty itp. (Które są umowami interfejsowymi, podobnie jak metody programowania). Tam zwykle obowiązuje zasada, że ​​nadawca powinien możliwie ściśle przestrzegać protokołu, a odbiorca powinien starać się pracować z tym, co nadchodzi.

Musisz więc zdecydować: czy naprawdę zadaniem metody bibliotecznej jest egzekwowanie dość dużych zasad, takich jak nullobsługa? Zwłaszcza jeśli Twoja biblioteka jest używana również przez inne osoby (które mogą mieć inne zasady).

Prawdopodobnie popełniłbym błąd po stronie dopuszczania nullwartości, definiowania i dokumentowania ich semantyki (tj. null = empty arrayW tym przypadku), i nie rzucałbym wyjątku, chyba że 99% twojego innego podobnego kodu (kodu w stylu „biblioteki”) robi to w drugą stronę 'okrągły.

AnoE
źródło
1
„Czy naprawdę zadaniem metody bibliotecznej jest egzekwowanie dość dużych zasad, takich jak obsługa zerowa?” - w tym myśleniu kłamią twierdzenia i tryby debugowania, które pozwalają ci pomóc w egzekwowaniu polityki, a nie w rzeczywistości egzekwowanie jej, jeśli tego właśnie chcesz. Więc jeśli zamierzasz przełknąć nullzasadę bycia liberalnym w tym, co akceptujesz, wtedy logowanie lub nawet rzucanie byłoby pomocne dla ludzi, którzy zamierzają być surowi w tym, co przekazują, ale którzy pomieszali zarówno swój kod, jak i ich testy jednostkowe w zakresie, w jakim i nulltak zdają .
Steve Jessop
@ SteveJessop, tak naprawdę nie wiem, czy kłócisz się z duchem mojej odpowiedzi, czy też kontynuujesz. To powiedziawszy, nie sugeruję po prostu przełykać null, ale deklaruję otwarcie w umowie metody, że jest to dopuszczalne (lub nie, w zależności od rozwiązania, jakie pojawi się OP), a następnie pytanie, czy zgłosić wyjątek (lub nie) rozwiązuje się.
AnoE