Anty-wzór

Czytałem w tym poście na temat anty-wzorca for-if i nie jestem pewien, czy rozumiem, dlaczego jest to anty-wzorzec.

foreach (string filename in Directory.GetFiles("."))
{
    if (filename.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))
    {
        return new StreamReader(filename);
    }
}

Pytanie 1:

Czy to z powodu return new StreamReader(filename);wewnątrz for loop? lub fakt, że forw tym przypadku nie potrzebujesz pętli?

Jak zauważył autor bloga, mniej szaloną wersją tego jest:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

oba cierpią z powodu wyścigu, ponieważ jeśli plik zostanie usunięty przed utworzeniem StreamReader, otrzymasz FileNotFoundException.

Pytanie 2:

Aby naprawić drugi przykład, czy napisałbyś go ponownie bez instrukcji if, a zamiast tego StreamReaderotoczyłeś blokiem try-catch, a jeśli wrzuci to FileNotFoundException, odpowiednio poradzisz sobie z tym catchblokiem?

c# anti-patterns

zobacz Omów ten $ {blog}

— komnata

@amon - Dziękuję, ale nie robię żadnych dodatkowych uwag, staram się tylko zrozumieć, co mówi artykuł i jak go poprawić.

Nie zastanawiałbym się tak długo. Plakat na blogu tworzy idiotyczny kod, którego nikt nie pisze, nadaje mu nazwę i nazywa go anty-wzorem. Oto kolejny: „Nazywam to bezcelowym wzorem przydziału: x = x; Cały czas to robię. Głupio. Myślę, że napiszę o tym bloga”.

— Martin Maat

To fix the second example, would you re-write it without the if statement, and instead surround the StreamReader with a try-catch block, and if it throws a FileNotFoundException you handle it in the catch block accordingly?

- Tak, właśnie to bym zrobił. Rozwiązanie stanu wyścigu jest ważniejsze niż pewne pojęcie „wyjątków jako kontroli” i rozwiązuje je elegancko i czysto.

— Robert Harvey,

Co robi, jeśli żaden z plików nie spełnia podanych kryteriów? Czy powróci null? Zazwyczaj możesz użyć LINQ, aby wyczyścić kod, który wygląda jak kod z twojego przykładu:

return Directory.GetFiles(".").FirstOrDefault(fileName => fileName.Equals("desktop.ini", StringComparison.OrdinalIgnoreCase))?.Select(fileName => new StreamReader(filename));

Zwróć uwagę na ?.operatora między dwiema wywołaniami LINQ. Ludzie mogą również argumentować, że takie tworzenie obiektów nie jest najwłaściwszym zastosowaniem LINQ, ale myślę, że tutaj jest w porządku. To nie jest odpowiedź na twoje pytanie, ale dygresje z jednej strony.

— Panzercrisis,

Odpowiedzi:

Jest to antypattern, ponieważ przyjmuje postać:

loop over a set of values
   if current value meets a condition
       do something with value
   end
end

i można go zastąpić

do something with value

Klasycznym przykładem tego jest kod taki jak:

for (var i=0; i < 5; i++)
{
    switch (i)
        case 1:
            doSomethingWith(1);
            break;
        case 2:
            doSomethingWith(2);
            break;
        case 3:
            doSomethingWith(4);
            break;
        case 4:
            doSomethingWith(4);
            break;
    }
}

Gdy następujące działania działają dobrze:

doSomethingWith(1);
doSomethingWith(2);
doSomethingWith(3);
doSomethingWith(4);

Jeśli zauważysz, że wykonujesz pętlę i iflub switch, zatrzymaj się i pomyśl o tym, co robisz. Czy nadmiernie komplikujesz rzeczy i czy cała pętla i test mogą zostać zastąpione prostą linią „po prostu zrób to”? Czasami jednak okaże się, że musisz wykonać tę pętlę (na przykład więcej niż jeden element może pasować do jednego warunku), w którym to przypadku wzór jest w porządku.

Dlatego jest to anty-wzorzec: bierze wzorzec „zapętlić i przetestować” i nadużywa go.

Jeśli chodzi o twoje drugie pytanie: tak. Wzorzec „spróbuj zrobić” jest bardziej niezawodny niż wzorzec „test to zrób” w każdej sytuacji, w której kod nie jest jedynym wątkiem na całym urządzeniu, który może zmienić stan testowanego elementu.

Problem z tym kodem:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

jest to, że w czasie pomiędzy File.Existsi StreamReaderpróbuje otworzyć ten plik, inny wątek lub proces może usunąć plik. Otrzymasz więc wyjątek. Dlatego ten wyjątek musi być chroniony przez coś takiego:

try
{
    return new StreamReader("desktop.ini");
}
catch (FileNotFoundException)
{
    return null; // or whatever
}

@Flater podnosi dobry punkt? Czy to samo jest antypatternem? Czy używamy wyjątków jako przepływu kontroli?

Jeśli kod odczytuje coś takiego:

try
{
    if (!File.Exists("desktop.ini")
    {
        throw new IniFileMissingException();
        return new StreamReader("desktop.ini");
    }
}
catch (IniFileMissingException)
{
    return null;
}

wtedy rzeczywiście użyłbym wyjątków jako uwielbionego goto i rzeczywiście byłby to anty-wzór. Ale w tym przypadku pracujemy właśnie nad niepożądanym zachowaniem tworzenia nowego strumienia, więc nie jest to przykład tego anty-wzorca. Ale jest to przykład obejścia tego anty-wzoru.

Oczywiście tak naprawdę chcemy bardziej eleganckiego sposobu tworzenia strumienia. Coś jak:

return TryCreateStream("desktop.ini", out var stream) ? stream : null;

I zaleciłbym zawinięcie tego try catchkodu w taką metodę narzędziową, jeśli często używasz tego kodu.

— David Arno
źródło

@ Później zgadzam się, że nie jest idealny (choć zaprzeczam, że jest to przykład anty-wzorca, o którym mówi ta odpowiedź). Kod powinien pokazywać swoje zamiary, a nie mechanikę. Tak więc faworyzuję coś takiego jak Scala Try, np. return Try(() => new StreamReader("desktop.ini")).OrElse(null);Ale C # nie obsługuje tej konstrukcji (bez użycia biblioteki innej firmy), więc musimy pracować z niezgrabną wersją.

— David Arno,

@ Później całkowicie się zgadzam, że wyjątki powinny być wyjątkowe. Ale rzadko tak jest. Na przykład plik nieistniejący nie jest niczym wyjątkowym, ale File.Openwyrzuci go, jeśli nie może go znaleźć. Ponieważ mamy już wyjątkowy wyjątek, wychwytywanie go jako część przepływu sterowania jest koniecznością (chyba że ktoś chce po prostu pozwolić aplikacji na awarię).

— David Arno,

@ Flater: drugi przykładowy kod jest właściwym sposobem na otwarcie pliku. Wszystko inne wprowadza warunek wyścigu. Nawet jeśli sprawdzisz istnienie plików, między tym sprawdzeniem a faktycznym otwarciem, coś może sprawić, że plik będzie dla ciebie niedostępny, a wywołanie Open () wybuchnie. Tak czy inaczej musisz być przygotowany na wyjątek. Więc równie dobrze możesz po prostu nie zawracać sobie głowy sprawdzaniem i po prostu spróbuj go otworzyć. Wyjątki to narzędzia, które pomagają nam robić rzeczy, a ich stosowanie nie powinno być postrzegane jako dogmat religijny.

— whatsisname

@ Flater: jakikolwiek sposób, aby uniknąć próbowania / złapania operacji na plikach, wprowadza warunki wyścigu. System plików, w którym chcesz pisać, może stać się niedostępny na chwilę przed wywołaniem File.Create, co powoduje, że wszelkie sprawdzanie jest nieprawidłowe.

— whatsisname

@ Flater „ Skutecznie argumentujesz, że każde wywołanie metody wymaga typu zwrotu… który skutecznie próbuje odkryć wyjątki w inny sposób ”. Poprawny. Chociaż to nowe odkrycie nie jest moje. Od lat jest używany w językach funkcjonalnych. Na ogół unikają one wszystkich „wyjątków jako kwestii przepływu kontroli” (które mylnie twierdzicie, że są anty-wzorcem w jednym oddechu, a następnie argumentują na korzyść w następnym), używając typów unii, np. W tym przypadku użylibyśmy Maybe<Stream>typu zwraca, nothingjeśli plik nie istnieje, i strumień, jeśli tak jest.

— David Arno,

Pytanie 1: (czy to antipattern jest w pętli for)

Tak, ponieważ nie musisz samodzielnie wyszukiwać elementów przechowywanych w podsystemie z zapytaniami.

W skrócie system plików, podobnie jak baza danych, może odpowiadać na zapytania. Podczas interakcji z podsystemem, którego dotyczy zapytanie, mamy fundamentalny wybór, czy chcemy, aby taki podsystem wyliczył nam jego zawartość, aby wykonać dopasowanie poza podsystemem, czy też skorzystać z jego natywnych funkcji zapytań.

Udawajmy przez chwilę, że szukasz rekordu w bazie danych zamiast pliku w katalogu w systemie plików. Wolałbyś zobaczyć

SELECT * FROM SomeTable;

a następnie w pętli (np. w C #) nad zwróconym kursorem szukając ID = 100, lub pozwalając podsystemowi zapytań zrobić, co w jego mocy, aby znaleźć dokładnie to, czego szukasz?

SELECT * FROM SomeTable WHERE ID = 100;

Powinienem pomyśleć, że większość z nas słusznie wybrałaby, aby podsystem wykonał dokładne zapytanie. Alternatywą są potencjalnie liczne podróże w obie strony z podsystemem, nieefektywne testowanie równości oraz rezygnacja z używania indeksów lub innych akceleratorów wyszukiwania, które zapewniają nam zarówno bazy danych, jak i systemy plików.

Pytanie 2: Aby naprawić drugi przykład, czy napisałbyś go ponownie bez instrukcji if, a zamiast tego otoczyłeś StreamReader blokiem try-catch, a jeśli zgłosi wyjątek FileNotFoundException, odpowiednio poradzisz sobie z nim w bloku catch?

Tak, ponieważ tak właśnie działa ten konkretny interfejs API - to nie jest tak naprawdę nasz wybór, ponieważ jest to funkcja biblioteki. Sprawdzenie if przed wywołaniem nie oferuje żadnej dodatkowej wartości: i tak musimy użyć try / catch, ponieważ (1) mogą wystąpić inne błędy poza FileNotFound i (2) warunek wyścigu.

— Erik Eidt
źródło

Pytanie 1:

Czy to z powodu zwrócenia nowego StreamReadera (nazwa pliku); wewnątrz pętli for? lub fakt, że w tym przypadku nie potrzebujesz pętli for?

Czytnik strumieniowy nie ma z tym nic wspólnego. Anty-wzór wyłania się z powodu wyraźnego konfliktu pomiędzy intencją foreacha if:

Jaki jest cel tego foreach?

Zakładam, że twoja odpowiedź będzie taka: „Chcę wielokrotnie wykonywać określony fragment kodu”

Ile plików zamierzasz przetworzyć?

Ponieważ możesz mieć tylko jedną konkretną nazwę pliku (łącznie z rozszerzeniem) w określonym folderze, dowodzi to, że Twój kod ma znaleźć jeden odpowiedni plik.

Potwierdza to również fakt, że zwracasz wartość natychmiast. Tak naprawdę nie obchodzi cię drugi mecz, nawet gdyby miał on istnieć.

Są sytuacje, w których nie jest to anty-wzór.

Jeśli zajrzysz również do podkatalogów ( Directory.GetFiles(".", SearchOption.AllDirectories)), możesz znaleźć więcej niż jeden plik o tej samej nazwie (łącznie z rozszerzeniem)
Jeśli szukasz częściowych dopasowań nazw plików (np. Każdego pliku, którego nazwa zaczyna się od "Test_", lub każdego "*.zip"pliku.

Zauważ, że oba te przypadki wymagałyby przetworzenia wielu dopasowań i dlatego nie zwracałyby natychmiast wartości.

Pytanie 2:

Aby naprawić drugi przykład, czy napisałbyś go ponownie bez instrukcji if, a zamiast tego otoczyłeś StreamReader blokiem try-catch, a jeśli zgłosi wyjątek FileNotFoundException, odpowiednio poradzisz sobie z nim w bloku catch?

Wyjątki są drogie. Nigdy nie należy ich używać zamiast właściwej logiki przepływu. Wyjątki są, ponieważ ich nazwa sugeruje wyjątkowe okoliczności.

Z tego powodu nie należy usuwać pliku if.

Zgodnie z odpowiedzią na SoftwareEngineering.SE :

Zasadniczo stosowanie wyjątków w przepływie kontrolnym jest anty-wzorcem, z kaszlem wyjątkowym zależnym od konkretnej sytuacji i języka.

Krótko podsumowując, dlaczego ogólnie jest to anty-wzór:

Wyjątki stanowią w istocie wyrafinowane wypowiedzi GOTO

Programowanie z wyjątkami prowadzi zatem do trudniejszego do odczytania i zrozumienia kodu

Większość języków ma istniejące struktury kontrolne zaprojektowane w celu rozwiązywania problemów bez użycia wyjątków

Argumenty dotyczące wydajności wydają się być dyskusyjne w przypadku nowoczesnych kompilatorów, które mają tendencję do optymalizacji przy założeniu, że wyjątki nie są używane do sterowania przepływem.

Przeczytaj dyskusję na wiki Warda, aby uzyskać więcej szczegółowych informacji.

To, czy chcesz zawinąć to w try / catch, zależy w dużej mierze od twojej sytuacji:

Jak prawdopodobne jest, że spotkasz się z wyścigiem?
Czy jesteś w stanie poradzić sobie z tą sytuacją, czy chcesz, aby problem ten pojawił się u użytkownika, ponieważ nie wiesz, jak sobie z tym poradzić.

Nic nie jest nigdy kwestią „zawsze używaj”. Aby udowodnić swój punkt widzenia:

Oddzielne badania wykazały, że mniej prawdopodobne jest, że odniesiesz obrażenia, gdy będziesz nosić kask ochronny, okulary ochronne i kamizelki kuloodporne.

Dlaczego więc nie wszyscy nosimy ten sprzęt bezpieczeństwa przez cały czas?

Prosta odpowiedź polega na tym, że noszenie ich ma wady:

Kosztuje pieniądze

Sprawia, że Twój ruch jest bardziej niewygodny

Noszenie go może być dość ciepłe.

Teraz do czegoś dochodzimy: są plusy i minusy . Innymi słowy, sensowne jest noszenie tego sprzętu tylko w przypadkach, gdy profesjonaliści przeważają nad wadami.

Pracownicy budowlani są znacznie bardziej narażeni na obrażenia podczas pracy. Korzystają z kasku ochronnego.

Z drugiej strony pracownicy biurowi mają znacznie mniejsze szanse na odniesienie obrażeń. Hełmy ochronne nie są tego warte.

Członek zespołu SWAT jest znacznie bardziej narażony na zastrzelenie w porównaniu z pracownikiem biurowym.

Czy powinieneś zawinąć połączenie w try / catch? To bardzo zależy od tego, czy korzyści z tego wynikające przeważają koszty jego wdrożenia.

Pamiętaj, że inni mogą argumentować, że wystarczy kilka naciśnięć klawiszy, aby go owinąć, więc oczywiście należy to zrobić. Ale to nie jest cały argument:

Musisz zdecydować, co zrobić, gdy złapiesz wyjątek.
Jeśli istnieje wiele różnych wywołań do różnych plików w całej bazie kodu, decyzja o zawinięciu jednego w try / catch na ogół oznacza, że musisz owinąć wszystkie te przypadki. Może to mieć ogromny wpływ na nakład pracy związany z jego wdrożeniem.
Jest całkiem możliwe, że celowo chcą nie obsłużyć wyjątek.
- Pamiętaj, że aplikacja będzie musiała obsłużyć wyjątek w pewnym momencie, ale niekoniecznie natychmiast po zgłoszeniu wyjątku.

Więc wybór należy do ciebie. Czy jest z tego korzyść? Czy uważasz, że poprawia ona aplikację, bardziej niż kosztowne wdrożenie?

Aktualizacja - od komentarza, który napisałem, do drugiej odpowiedzi, ponieważ uważam, że jest to również istotne dla ciebie:

Bardzo zależy to od otaczającego kontekstu.

Jeśli otwarcie czytnika strumieniowego jest poprzedzone if(!File.Exists) File.Create(), to brak pliku podczas otwierania czytnika strumieniowego jest rzeczywiście wyjątkowy .

Jeśli nazwa pliku została wybrana z listy istniejących plików, jej nagły brak znów jest wyjątkowy .

Jeśli pracujesz z ciągiem, który nie został jeszcze przetestowany względem katalogu; wtedy brak pliku jest wynikiem całkowicie logicznym, a zatem nie wyjątkowym .

— Flater
źródło