Czy nadmiarowe sprawdzanie stanu jest złym stylem?

10

Często przechodzę do pozycji w moim kodzie, gdzie ciągle sprawdzam określony stan.

Chcę dać ci mały przykład: załóżmy, że istnieje plik tekstowy zawierający linie zaczynające się od „a”, linie rozpoczynające się od „b” i inne linie, a tak naprawdę chcę pracować tylko z dwoma pierwszymi rodzajami linii. Mój kod wyglądałby mniej więcej tak (używając Pythona, ale czytałbym go jako pseudokod):

# ...
clear_lines() # removes every other line than those starting with "a" or "b"
for line in lines:
    if (line.startsWith("a")):
        # do stuff
    elif (line.startsWith("b")):
        # magic
    else:
        # this else is redundant, I already made sure there is no else-case
        # by using clear_lines()
# ...

Możesz sobie wyobrazić, że nie sprawdzę tutaj tylko tego warunku, ale może także w innych funkcjach i tak dalej.

Czy myślisz o tym jako o szumie, czy może to wnosi dodatkową wartość do mojego kodu?

marktani
źródło
5
Zasadniczo chodzi o to, czy kodujesz defensywnie. Czy często edytujesz ten kod? Czy prawdopodobne jest, że będzie to część systemu, który musi być wyjątkowo niezawodny? Nie widzę wielkiej szkody we wpychaniu assert()tam, aby pomóc w testowaniu, ale poza tym jest to prawdopodobnie nadmierne. To powiedziawszy, będzie się różnić w zależności od sytuacji.
Latty
Twoja sprawa „else” jest w zasadzie martwym / nieosiągalnym kodem. Sprawdź, czy nie ma wymagań systemowych, które by tego zabraniały.
NWS
@NWS: czy mówisz, że powinienem zachować sprawę else? Przepraszam, nie rozumiem cię całkowicie.
marktani,
2
niezbyt związane z pytaniem - ale uczyniłbym to „twierdzenie” niezmiennikiem - co wymagałoby nowej klasy „Linia” (być może z klasami pochodnymi dla A i B), zamiast traktowania linii jako ciągów znaków i mówienia im, co reprezentują z zewnątrz. Z przyjemnością omówię
MattDavey,
miałeś na myśli elif (line.startsWith("b"))? przy okazji, możesz bezpiecznie usunąć te otaczające nawiasy pod warunkiem, że nie są idiomatyczne w Pythonie.
tokland

Odpowiedzi:

14

Jest to wyjątkowo powszechna praktyka, a sposobem radzenia sobie z nią są filtry wyższego rzędu .

Zasadniczo przekazujesz funkcję do metody filter wraz z listą / sekwencją, według której chcesz filtrować, a wynikowa lista / sekwencja zawiera tylko te elementy, które chcesz.

Nie znam składni Pythona (chociaż zawiera taką funkcję, jak widać w powyższym linku), ale w c # / f # wygląda to tak:

do#:

var linesWithAB = lines.Where(l => l.StartsWith("a") || l.StartsWith("b"));
foreach (var line in linesWithAB)
{
    /* line is guaranteed to ONLY start with a or b */
}

f # (zakłada, że ​​jest niepoliczalny, w przeciwnym razie użyłby List.filter):

let linesWithAB = lines
    |> Seq.filter (fun l -> l.StartsWith("a") || l.StartsWith("b"))

for line in linesWithAB do
    /* line is guaranteed to ONLY start with a or b */

Tak więc, żeby być jasnym: jeśli używasz sprawdzonego kodu / wzorców, jest to zły styl. To i mutowanie listy w pamięci w sposób, w jaki wyglądasz za pomocą clear_lines (), traci bezpieczeństwo wątków i wszelkie nadzieje na paralelizm, który mógłbyś mieć.

Steven Evers
źródło
3
Jako notatkę, składnia Pythona do tego byłoby wyrażenie generator: (line for line in lines if line.startswith("a") or line.startswith("b")).
Latty
1
+1 za wskazanie, że (niepotrzebne) imperatywne wdrożenie clear_linesjest naprawdę złym pomysłem. W Pythonie prawdopodobnie użyłbyś generatorów, aby uniknąć ładowania całego pliku do pamięci.
tokland
Co się stanie, gdy plik wejściowy jest większy niż dostępna pamięć?
Blrfl,
@Blrfl: cóż, jeśli generator terminów jest spójny między c # / f # / python, to co @tokland i @Lattyware przekłada się na plon c / f # i / lub plon! sprawozdania. Jest to trochę bardziej oczywiste w moim przykładzie f #, ponieważ Seq.filter można zastosować tylko do kolekcji IEnumerable <T>, ale oba przykłady kodu będą działać, jeśli linesjest generowaną kolekcją.
Steven Evers,
@mcwise: Kiedy zaczniesz patrzeć na wszystkie inne dostępne funkcje, które działają w ten sposób, zaczyna się naprawdę seksownie i niesamowicie ekspresyjnie, ponieważ można je wszystkie połączyć i połączyć. Spójrz na skip, take, reduce( aggregatew .NET), map( selectw .NET), a tam więcej, ale to jest naprawdę dobry początek.
Steven Evers,
14

Niedawno musiałem wdrożyć programator oprogramowania układowego w formacie S-record Motoroli , bardzo podobny do tego, co opisujesz. Ponieważ mieliśmy niewielką presję czasu, mój pierwszy szkic zignorowałem zwolnienia i wprowadziłem uproszczenia w oparciu o podzbiór, którego faktycznie potrzebowałem w mojej aplikacji. Z łatwością przeszedł moje testy, ale zawiódł, gdy tylko ktoś go spróbował. Nie było pojęcia, na czym polega problem. Przeszedł całą drogę, ale na końcu się nie udało.

Nie miałem więc wyboru, jak wdrożyć wszystkie zbędne kontrole, aby zawęzić problem. Po tym czasie znalezienie problemu zajęło mi około dwóch sekund.

Zajęło mi to może dodatkowe dwie godziny, aby zrobić to we właściwy sposób, ale zmarnowałem także dzień innych ludzi na rozwiązywanie problemów. Bardzo rzadko kilka cykli procesora jest wartych jednego dnia zmarnowanego rozwiązywania problemów.

To powiedziawszy, jeśli chodzi o odczytywanie plików, często korzystne jest zaprojektowanie oprogramowania do pracy z odczytem i przetwarzaniem go pojedynczo, zamiast wczytywania całego pliku do pamięci i przetwarzania go w pamięci. W ten sposób nadal będzie działać na bardzo dużych plikach.

Karl Bielefeldt
źródło
„Bardzo rzadko kilka cykli procesora jest wartych jednego dnia zmarnowanego rozwiązywania problemów”. Dzięki za odpowiedź, masz rację.
marktani,
5

Możesz zgłosić wyjątek w elseprzypadku. W ten sposób nie jest to zbędne. Wyjątki to rzeczy, które nie powinny się zdarzyć, ale i tak są sprawdzane.

clear_lines() # removes every other line than those starting with "a" or "b"
for line in lines:
    if (line.startsWith("a)):
        # do stuff
    if (line.startsWith("b")):
        # magic
    else:
        throw BadLineException
# ...
Tulains Córdova
źródło
Twierdziłbym, że ten ostatni jest złym pomysłem, ponieważ jest mniej wyraźny - jeśli później zdecydujesz się dodać "c", może być mniej jasny.
Latty
Pierwsza sugestia ma sens ... druga (zakładając „b”) jest złym pomysłem
Andrew
@Lattyware Poprawiłem odpowiedź. Dziękuję za komentarze.
Tulains Córdova,
1
@Andrew poprawiłem odpowiedź. Dziękuję za komentarze.
Tulains Córdova,
3

Przy projektowaniu na podstawie umowy zgaduje się, że każda funkcja musi wykonywać swoją pracę zgodnie z opisem w dokumentacji. Tak więc każda funkcja ma listę warunków wstępnych, czyli warunków na wejściach funkcji, a także warunków końcowych, czyli warunków wyjścia funkcji.

Funkcja musi gwarantować swoim klientom, że jeśli dane wejściowe spełniają warunki wstępne, to wynik będzie taki, jak opisano w warunkach końcowych. Jeśli co najmniej jeden z warunków wstępnych nie jest przestrzegany, funkcja może zrobić, co chce (zawiesić się, zwrócić dowolny wynik, ...). Dlatego warunki wstępne i końcowe są semantycznym opisem funkcji.

Dzięki umowie funkcja ma pewność, że jej klienci używają jej poprawnie, a klient ma pewność, że funkcja poprawnie wykonuje swoje zadanie.

Niektóre języki obsługują kontrakty natywnie lub poprzez dedykowane ramy. Dla pozostałych najlepiej sprawdzać warunki wstępne i końcowe dzięki zapewnieniom, jak powiedział @Lattyware. Ale nie nazwałbym tego programowaniem defensywnym, ponieważ moim zdaniem koncepcja ta koncentruje się bardziej na ochronie przed wkładem użytkownika (człowieka).

Jeśli wykorzystasz kontrakty, możesz uniknąć warunku nadmiarowo sprawdzonego, ponieważ albo wywoływana funkcja działa doskonale i nie potrzebujesz podwójnego sprawdzania, albo wywoływana funkcja jest dysfunkcyjna, a funkcja wywołująca może zachowywać się tak, jak chce.

Najtrudniejsze jest zatem określenie, która funkcja jest odpowiedzialna za co i ścisłe udokumentowanie tych ról.

mgoeminne
źródło
1

Na początku nie potrzebujesz clear_lines (). Jeśli linia nie jest ani „a”, ani „b”, warunki warunkowe po prostu się nie uruchomią. Jeśli chcesz się pozbyć tych linii, ustaw inną w clear_line (). W tej chwili robisz dwa przejścia przez dokument. Jeśli pominiesz funkcję clear_lines () na początku i zrobisz to jako część pętli foreach, skrócisz czas przetwarzania o połowę.

To nie tylko zły styl, ale i złe obliczenia.

Inżynier świata
źródło
2
Może się zdarzyć, że te linie są używane do czegoś innego i należy się nimi zająć przed obsługą linii "a"/ "b". Nie mówię, że to prawdopodobne ( wyraźna nazwa oznacza, że ​​są odrzucane), tylko istnieje możliwość, że jest to potrzebne. Jeśli ten zestaw linii będzie wielokrotnie powtarzany w przyszłości, warto je wcześniej usunąć, aby uniknąć wielu bezcelowych iteracji.
Latty
0

Jeśli naprawdę chcesz coś zrobić, jeśli znajdziesz niepoprawny ciąg (na przykład wyjściowy tekst debugowania), powiedziałbym, że to absolutnie w porządku. Kilka dodatkowych wierszy i kilka miesięcy później, gdy przestanie działać z nieznanego powodu, możesz sprawdzić wyniki, aby dowiedzieć się, dlaczego.

Jeśli jednak można go po prostu zignorować lub wiesz na pewno, że nigdy nie otrzymasz niepoprawnego ciągu, to nie potrzebujesz dodatkowej gałęzi.

Osobiście jestem zawsze za wprowadzeniem przynajmniej danych wyjściowych śledzenia dla każdego nieoczekiwanego stanu - znacznie ułatwia życie, gdy masz błąd z dołączonym wyjściem, który mówi dokładnie, co poszło nie tak.

Bok McDonagh
źródło
0

... przypuśćmy, że istnieje plik tekstowy zawierający linie zaczynające się od „a”, linie rozpoczynające się od „b” i inne linie, a tak naprawdę chcę pracować tylko z dwoma pierwszymi rodzajami linii. Mój kod wyglądałby mniej więcej tak (używając Pythona, ale czytałbym go jako pseudokod):

# ...
clear_lines() # removes every other line than those starting with "a" or "b"
for line in lines:
    if ...

Nienawidzę if...then...elsekonstrukcji. Unikałbym całego problemu:

process_lines_by_first_character (lines,  
                                  'a' => { |line| ... a code ... },
                                  'b' => { |line| ... b code ... } )
Kevin Cline
źródło