Jest to drobny problem, ale za każdym razem, gdy muszę kodować coś takiego, powtarzanie przeszkadza mi, ale nie jestem pewien, czy którekolwiek z rozwiązań nie jest gorsze.
if(FileExists(file))
{
contents = OpenFile(file); // <-- prevents inclusion in if
if(SomeTest(contents))
{
DoSomething(contents);
}
else
{
DefaultAction();
}
}
else
{
DefaultAction();
}
- Czy istnieje nazwa dla tego rodzaju logiki?
- Czy jestem trochę za obsesyjna?
Jestem otwarty na sugestie złego kodu, choćby ze względu na ciekawość ...
coding-style
conditions
Benjol
źródło
źródło
DefaultAction
połączenia naruszają zasadę OSUSZANIAmake sure I have valid data for DoSomething(), and then DoSomething() with it. Otherwise, take DefaultAction()
. Szczegółowe szczegóły dotyczące upewnienia się, że masz dane do DoSomething (), są na niższym poziomie abstrakcji, a zatem powinny mieć inną funkcję. Ta funkcja będzie miała nazwę na wyższym poziomie abstrakcji, a jej implementacja będzie na niskim poziomie. Dobre odpowiedzi poniżej rozwiązują ten problem.Odpowiedzi:
Wyodrębnij go do oddzielnej funkcji (metody) i użyj
return
instrukcji:A może lepiej oddzielić pobieranie treści i ich przetwarzanie:
Aktualizacja:
Dlaczego nie wyjątki, dlaczego
OpenFile
nie zgłasza wyjątku IO:Myślę, że to naprawdę ogólne pytanie, a nie pytanie o IO pliku. Nazwy takie jak
FileExists
,OpenFile
może być mylące, ale jeśli je zastąpićFoo
,Bar
itp, - byłoby jasne, żeDefaultAction
można nazwać jak najczęściejDoSomething
, więc może to nie być przypadek wyjątkowy. Péter Török napisał o tym na końcu swojej odpowiedziDlaczego w drugim wariancie występuje trójskładnikowy operator warunkowy:
Gdyby istniał tag [C ++], napisałbym
if
oświadczenie z deklaracjącontents
części:Ale w przypadku innych (podobnych do C) języków
if(contents) ...; else ...;
jest to dokładnie to samo, co wyrażenie wyrażenia z potrójnym operatorem warunkowym, ale dłużej. Ponieważ główną częścią kodu byłaget_contents
funkcja, właśnie użyłem krótszej wersji (a także pominiętegocontents
typu). Tak czy inaczej, to jest poza tym pytaniem.źródło
Jeśli używany język programowania (0) powoduje zwarcie porównań binarnych (tzn. Jeśli nie wywołuje,
SomeTest
jeśliFileExists
zwraca false) i (1) przypisanie zwraca wartość (wynikOpenFile
jest przypisywany,contents
a następnie ta wartość jest przekazywana jako argument doSomeTest
), możesz użyć czegoś takiego jak poniżej, ale nadal zaleca się komentowanie kodu, zauważając, że singiel=
jest celowy.W zależności od tego, jak skomplikowany jest if, może być lepiej mieć zmienną flagową (która oddziela testowanie warunków powodzenia / niepowodzenia kodem, który obsługuje błąd
DefaultAction
w tym przypadku)źródło
if
Moim zdaniem dość obrzydliwe jest umieszczenie tak dużej ilości kodu w oświadczeniu.Poważniejsze niż powtarzanie wywołania DefaultAction jest sam styl, ponieważ kod jest pisany nieortogonalny (patrz ta odpowiedź z dobrych powodów, aby pisać ortogonalnie).
Aby pokazać, dlaczego kod nieortogonalny jest zły, rozważ oryginalny przykład, kiedy wprowadzono nowy wymóg, aby nie otwierać pliku, jeśli jest on przechowywany na dysku sieciowym. Cóż, moglibyśmy po prostu zaktualizować kod do następującego:
Ale pojawia się również wymóg, aby nie otwierać dużych plików o wielkości powyżej 2 Gb. Cóż, właśnie aktualizujemy:
Powinno być bardzo jasne, że taki styl kodu będzie ogromnym problemem w utrzymaniu.
Wśród odpowiedzi tutaj poprawnie napisanych ortogonalnie są drugi przykład Abyxa i odpowiedź Jana Hudca , więc nie powtórzę tego, po prostu zaznaczę , że dodanie dwóch wymagań w tych odpowiedziach byłoby po prostu
(lub
goto notexists;
zamiastreturn null;
), nie wpływając na żaden inny kod niż te dodane wiersze . Np. Ortogonalny.Podczas testowania ogólną zasadą powinno być testowanie wyjątków, a nie normalny przypadek .
źródło
Oczywiście:
Powiedziałeś, że jesteś otwarty nawet na złe rozwiązania, więc użycie zła goto się liczy, nie?
W rzeczywistości, w zależności od kontekstu, rozwiązanie to może być mniej złe niż zło wykonujące akcję dwukrotnie lub zła dodatkowa zmienna. Owinąłem ją w funkcję, ponieważ zdecydowanie nie byłaby OK w środku długiej funkcji (zwłaszcza ze względu na powrót w środku). Ale niż długa funkcja nie jest OK, kropka.
Kiedy masz wyjątki, będą łatwiejsze do odczytania, szczególnie jeśli możesz mieć OpenFile i DoSomething po prostu rzucać wyjątek, jeśli warunki nie są spełnione, więc nie potrzebujesz wyraźnych kontroli. Z drugiej strony w C ++, Javie i C # zgłaszanie wyjątku jest powolną operacją, więc od punktu wydajności goto jest nadal preferowane.
Uwaga na temat „zła”: C ++ FAQ 6.15 definiuje „zło” jako:
I dotyczy to
goto
w tym kontekście. Strukturyzowane konstrukcje kontroli przepływu są lepsze przez większość czasu, ale kiedy znajdziesz się w sytuacji, w której gromadzą zbyt wiele własnych zła, takie jak przypisanie warunku, zagnieżdżanie na głębokości ponad 3 poziomów, powielanie kodu lub długie warunki,goto
mogą po prostu się skończyć bycie mniej złym.źródło
for(;;) { if(!FileExists(file)) break; contents = OpenFile(file); if(!SomeTest(contents)) break; DoSomething(contents); return; } /* broken out */ DefaultAction();
goto
, ponieważ nadużywaszbreak
w sposób, którego nikt nie spodziewa się, że zostanie wykorzystany, więc ludzie czytający kod będą mieli więcej problemów ze zrozumieniem, gdzie prowadzi przerwa, niż goto, co mówi wprost. Poza tym używasz nieskończonej pętli, która będzie działać tylko raz, co będzie dość mylące. Niestety, teżdo { ... } while(0)
nie jest do końca czytelny, ponieważ widzisz tylko zabawny blok, gdy dojdziesz do końca, a C nie obsługuje przełamywania innych bloków (w przeciwieństwie do Perla)....
źródło
SomeTest
może mieć taką samą semantykę jak istnienie pliku, jeśliSomeTest
sprawdza typ pliku, na przykład sprawdza, czy .gif jest naprawdę plikiem GIF.contents && f(contents)
. Dwie funkcje, aby uratować jeszcze jedną?Jedna możliwość:
Oczywiście powoduje to, że kod jest nieco bardziej złożony w inny sposób. Jest to więc w dużej mierze pytanie dotyczące stylu.
Innym podejściem byłoby stosowanie wyjątków, np .:
Wygląda to na prostsze, ale ma zastosowanie tylko wtedy, gdy
DefaultAction()
pasuje do każdego z nichSomeTest()
jest oczywiście błędnym warunkiem, dlatego należy na niego rzucić wyjątek.źródło
(function () { ... })()
w JavaScript,{ flag = false; ... }
w C-jak itp.Jest to na wyższym poziomie abstrakcji:
I to uzupełnia szczegóły.
źródło
Niektórzy uważają, że takie podejście jest nieco ekstremalne, ale jest również bardzo czyste. Pozwól mi zilustrować w Pythonie:
Kiedy mówi, że funkcje powinny robić jedną rzecz, ma na myśli jedną rzecz.
processFile()
wybiera akcję na podstawie wyniku testu i to wszystko.fileMeetsTest()
łączy wszystkie warunki testu i to wszystko.contentsTest()
przenosi pierwszą linięfirstLineTest()
i to wszystko, co robi.Wygląda na wiele funkcji, ale brzmi praktycznie jak prosty angielski:
To prawda, że to trochę nieporadne, ale należy pamiętać, że jeśli opiekun nie dba o szczegóły, może przestać czytać po zaledwie 4 liniach kodu
processFile()
i nadal będzie miał dobrą znajomość wysokiego poziomu na temat funkcji.źródło
W odniesieniu do tego, co to się nazywa, może łatwo rozwinąć się w anty-wzór grotu strzałki, gdy Twój kod rośnie, aby obsłużyć więcej wymagań (jak pokazano w odpowiedzi podanej na https://softwareengineering.stackexchange.com/a/122625/33922 ) i następnie wpada w pułapkę posiadania ogromnych sekcji kodów z zagnieżdżonymi instrukcjami warunkowymi przypominającymi strzałkę.
Zobacz linki takie jak;
http://codinghorror.com/blog/2006/01/flattening-arrow-code.html
http://lostechies.com/chrismissal/2009/05/27/anti-patterns-and-worst-practices-the-arrowhead-anti-pattern/
W Google jest wiele innych informacji na temat tego i innych anty-wzorów.
Oto kilka świetnych wskazówek, które Jeff udziela na swoim blogu;
Zobacz także niektóre komentarze na blogu Jeffa dotyczące sugestii Steve'a McConnellsa dotyczących wczesnych powrotów;
Zawsze zapisywałem się na teorię 1 wejście / wyjście na funkcję ze względu na to, czego nauczono mnie 15 lat temu. Wydaje mi się, że dzięki temu kod jest o wiele łatwiejszy do odczytania i, jak wspomniałeś, łatwiejszy do utrzymania
źródło
Jest to zgodne z zasadami DRY, no-goto i no-multiple-return, moim zdaniem jest skalowalne i czytelne:
źródło
if
s wewnątrz innychif
s. Ponadto kod do obsługi nieudanej instrukcji case (DefaultAction()
) znajduje się tylko w jednym miejscu i do celów debugowania kod nie przeskakuje funkcji pomocniczych, a dodawanie punktów przerwania do linii, w którychsuccess
zmieniona jest zmienna, może szybko pokazać, które testy przeszły (powyżej wyzwalanego punkt przerwania), a które nie zostały przetestowane (poniżej).success
ok_so_far
Wyodrębnię go do oddzielnej metody, a następnie:
co również pozwala
następnie prawdopodobnie możesz usunąć
DefaultAction
połączenia i pozostawić wykonanieDefaultAction
dla dzwoniącego:Lubię też podejście Jeanne Pindar .
źródło
W tym konkretnym przypadku odpowiedź jest dość łatwa ...
Występuje warunek wyścigu pomiędzy
FileExists
iOpenFile
: co się stanie, jeśli plik zostanie usunięty?Jedynym rozsądnym sposobem rozwiązania tego konkretnego przypadku jest pominięcie
FileExists
:To starannie rozwiązuje ten problem i sprawia, że kod jest czystszy.
Ogólnie: spróbuj przemyśleć problem i opracować inne rozwiązanie, które całkowicie go omija.
źródło
Inną możliwością, jeśli nie lubisz widzieć zbyt wielu innych, jest całkowite zrezygnowanie z użycia else i dodanie dodatkowej instrukcji return. W przeciwnym razie jest to zbyteczne, chyba że potrzebujesz bardziej złożonej logiki, aby ustalić, czy istnieją więcej niż dwie możliwości działania.
Zatem twój przykład może wyglądać następująco:
Osobiście nie mam nic przeciwko użyciu klauzuli else , która wyraźnie określa, w jaki sposób logika powinna działać, a zatem poprawia czytelność twojego kodu. Niektóre narzędzia upiększania kodu wolą jednak upraszczać od pojedynczej instrukcji if , aby zniechęcić do logiki zagnieżdżania.
źródło
Przypadek pokazany w przykładowym kodzie można zwykle sprowadzić do pojedynczej
if
instrukcji. W wielu systemach funkcja otwierania pliku zwróci niepoprawną wartość, jeśli plik jeszcze nie istnieje. Czasami jest to zachowanie domyślne; innym razem należy to określić za pomocą argumentu. Oznacza to, żeFileExists
test można przerwać, co może również pomóc w warunkach wyścigu wynikających z usunięcia pliku między testem istnienia a otwarciem pliku.Nie rozwiązuje to bezpośrednio problemu mieszania poziomów abstrakcji, ponieważ całkowicie pomija kwestię wielu, niemożliwych do łańcuchowania testów, chociaż rezygnacja z testu istnienia pliku nie jest niezgodna z oddzielaniem poziomów abstrakcji. Zakładając, że nieprawidłowe uchwyty plików są równoważne „fałszowi”, a uchwyty plików zamykają się, gdy wykraczają poza zakres:
źródło
Jednak zgadzam się z frozenkoi, jednak w przypadku C #, pomyślałem, że pomogłoby to postępować zgodnie ze składnią metod TryParse.
źródło
Twój kod jest brzydki, ponieważ robisz za dużo w jednej funkcji. Chcesz przetworzyć plik lub podjąć domyślną akcję, więc zacznij od stwierdzenia, że:
Programiści Perla i Ruby piszą
processFile(file) || defaultAction()
Teraz idź napisz ProcessFile:
źródło
Oczywiście możesz posunąć się tak daleko tylko w takich scenariuszach, ale jest na to sposób:
Możesz chcieć dodatkowe filtry. Następnie zrób to:
Chociaż może to mieć również sens:
Które jest najlepsze? To zależy od rzeczywistego problemu , przed którym stoisz.
Ale rzeczą, którą należy zabrać, jest: możesz wiele zrobić z kompozycją i polimorfizmem.
źródło
Co jest nie tak z tym, co oczywiste
Wydaje mi się to dość standardowe? Dla tego rodzaju wielkiej procedury, w której musi wydarzyć się wiele drobiazgów, których żadna z nich nie zapobiegłaby. Wyjątki sprawiają, że jest to nieco czystsze, jeśli jest to opcja.
źródło
Wiem, że to stare pytanie, ale zauważyłem wzorzec, o którym nie wspomniano; głównie, ustawiając zmienną w celu późniejszego określenia metody, którą chcesz wywołać (poza if ... else ...).
Jest to kolejny punkt widzenia, aby ułatwić obsługę kodu. Pozwala również na określenie, kiedy można dodać kolejną metodę, która ma zostać wywołana, lub zmienić odpowiednią metodę, która musi zostać wywołana w określonych sytuacjach.
Zamiast wymieniać wszystkie wzmianki o metodzie (i być może brakuje niektórych scenariuszy), wszystkie są wymienione na końcu bloku if ... else ... i są łatwiejsze do odczytania i zmiany. Zazwyczaj używam tego, gdy na przykład można wywołać kilka metod, ale w zagnieżdżonym, jeśli ... w przeciwnym razie ... można wywołać metodę w kilku dopasowaniach.
Jeśli ustawisz zmienną, która definiuje stan, możesz mieć wiele głęboko zagnieżdżonych opcji i aktualizować stan, gdy coś ma zostać wykonane (lub nie).
Można to wykorzystać jak w przykładzie zadanym w pytaniu, w którym sprawdzamy, czy wystąpiło „DoSomething”, a jeśli nie, wykonaj domyślną akcję. Możesz też podać stan dla każdej metody, którą chcesz wywołać, ustawić, gdy ma to zastosowanie, a następnie wywołać odpowiednią metodę poza ...
Na końcu zagnieżdżonych instrukcji if ... else ... sprawdzasz stan i odpowiednio postępujesz. Oznacza to, że potrzebujesz tylko jednej wzmianki o metodzie zamiast wszystkich lokalizacji, w których powinna zostać zastosowana.
źródło
Aby zmniejszyć zagnieżdżone JEŻELI:
1 / wczesny powrót;
2 / wyrażenie złożone (rozpoznawanie zwarcia)
Zatem twój przykład może zostać zmieniony w następujący sposób:
źródło
Widziałem wiele przykładów z „return”, których też używam, ale czasami chcę uniknąć tworzenia nowych funkcji i zamiast tego użyć pętli:
Jeśli chcesz pisać mniej linii lub nienawidzisz nieskończonych pętli tak jak ja, możesz zmienić typ pętli na „do ... while (0)” i uniknąć ostatniego „break”.
źródło
Co z tym rozwiązaniem:
Przyjąłem założenie, że OpenFile zwraca wskaźnik, ale może to również działać ze zwrotem typu wartości poprzez określenie pewnej wartości domyślnej, która nie podlega zwrotowi (kody błędów lub coś podobnego).
Oczywiście nie oczekuję żadnych możliwych działań za pomocą metody SomeTest na wskaźniku NULL (ale nigdy nie wiadomo), więc może to być również postrzegane jako dodatkowe sprawdzenie wskaźnika NULL dla wywołania SomeTest (zawartość).
źródło
Najwyraźniej najbardziej eleganckim i zwięzłym rozwiązaniem jest użycie makra preprocesora.
Co pozwala napisać piękny kod w ten sposób:
Może być trudno polegać na automatycznym formatowaniu, jeśli często używasz tej techniki, a niektóre środowiska IDE mogą krzyczeć na ciebie trochę za to, co błędnie podejrzewa, że jest zniekształcone. Jak mówi przysłowie, wszystko jest kompromisem, ale przypuszczam, że nie jest to zła cena, aby uniknąć zła wynikającego z powtarzania się kodu.
źródło
Ponieważ zadałeś pytanie z ciekawości, a twoje pytanie nie jest oznaczone określonym językiem (chociaż jasne jest, że masz na myśli języki imperatywne), warto dodać, że języki obsługujące leniwe ocenianie pozwalają na zupełnie inne podejście. W tych językach wyrażenia są oceniane tylko wtedy, gdy są potrzebne, więc można zdefiniować „zmienne” i używać ich tylko wtedy, gdy ma to sens. Na przykład w fikcyjnym języku z leniwymi
let
/in
strukturami zapominasz o kontroli przepływu i piszesz:źródło