Staram się stosować do czystego kodu wuja Boba, a konkretnie, aby metody były krótkie.
Nie mogę jednak skrócić tej logiki:
if (checkCondition()) {addAlert(1);}
else if (checkCondition2()) {addAlert(2);}
else if (checkCondition3()) {addAlert(3);}
else if (checkCondition4()) {addAlert(4);}
Nie mogę usunąć pozostałych i tym samym podzielić całej rzeczy na mniejsze części, ponieważ „else” w „else if” pomaga wydajności - ocena tych warunków jest droga i jeśli mogę uniknąć oceny poniższych warunków, spowoduję, że jeden z pierwszych to prawda, chcę ich uniknąć.
Nawet semantycznie rzecz biorąc, ocena następnego warunku, jeśli poprzednie zostało spełnione, nie ma sensu z biznesowego punktu widzenia.
edycja: To pytanie zostało zidentyfikowane jako możliwy duplikat eleganckich sposobów radzenia sobie, jeśli (jeśli inaczej) inaczej .
Uważam, że jest to inne pytanie (można to zobaczyć również poprzez porównanie odpowiedzi na te pytania).
- Moje pytanie dotyczy sprawdzenia, czy pierwszy warunek akceptacji szybko się kończy .
- Połączone pytanie stara się spełnić wszystkie warunki , aby coś zrobić. (lepiej widać w odpowiedzi na to pytanie: https://softwareengineering.stackexchange.com/a/122625/96955 )
źródło
Odpowiedzi:
Idealnie uważam, że powinieneś wyodrębnić swoją logikę, aby wprowadzić kod / numer alertu do własnej metody. Twój istniejący kod został zredukowany do
i masz GetConditionCode () enkapsuluje logikę sprawdzania warunków. Może także lepiej użyć Enum niż magicznej liczby.
źródło
addAlert
musi sprawdzać stan fałszywego alertuAlertCode.None
.Ważnym pomiarem jest złożoność kodu, a nie wielkość bezwzględna. Zakładając, że różne warunki są tak naprawdę pojedynczymi wywołaniami funkcji, podobnie jak działania nie są bardziej złożone niż to, co pokazałeś, powiedziałbym, że nie ma nic złego w kodzie. To jest już tak proste, jak to tylko możliwe.
Każda próba dalszego „uproszczenia” naprawdę skomplikuje sprawę.
Oczywiście możesz zastąpić
else
słowo kluczowe na,return
jak sugerowali inni, ale to tylko kwestia stylu, a nie zmiana złożoności.Na bok:
Moja ogólna rada brzmiałaby: nigdy nie upominaj się o żadnej zasadzie czystego kodu: większość porad dotyczących kodowania, które widzisz w Internecie, jest dobra, jeśli zastosujesz je w odpowiednim kontekście, ale radykalne zastosowanie tej samej porady wszędzie może wygrać wpis w IOCCC . Sztuka polega na tym, aby zawsze znaleźć równowagę, która pozwoli ludziom łatwo zrozumieć kod.
Używaj zbyt dużych metod, a jesteś wkręcony. Użyj zbyt małych funkcji, a będziesz wkręcony. Unikaj wyrażeń potrójnych, a jesteś przykręcony. Wszędzie używaj wyrażeń potrójnych, a ty jesteś przykręcony. Uświadom sobie, że są miejsca, które wymagają funkcji jednowierszowych i miejsca, które wymagają funkcji 50-liniowych (tak, istnieją!). Uświadom sobie, że są miejsca, które wymagają
if()
wyciągów i że są miejsca, które wymagają?:
operatora. Skorzystaj z pełnego arsenału, który jest do Twojej dyspozycji, i zawsze staraj się używać najlepiej dopasowanego narzędzia, jakie możesz znaleźć. I pamiętajcie, nie bądźcie religijni nawet na temat tej rady.źródło
else if
na wewnętrzny,return
po którym następuje prostyif
(usunięcieelse
), może utrudnić odczytanie kodu . Kiedy kod mówielse if
, od razu wiem, że kod w następnym bloku zostanie wykonany tylko wtedy, gdy poprzedni nie. Bez musów, bez zamieszania. Jeśli jest to zwykłyif
, może on wykonać lub nie, niezależnie od tego, czy poprzedni został wykonany. Teraz będę musiał poświęcić trochę wysiłku umysłowego, aby przeanalizować poprzedni blok, aby zauważyć, że kończy się on nareturn
. Wolę poświęcić ten wysiłek umysłowy na analizowanie logiki biznesowej.else if
tworzy jedną jednostkę semantyczną. (To niekoniecznie jest pojedyncza jednostka dla kompilatora, ale to jest w porządku.)...; return; } if (...
Nie; nie mówiąc już o tym, że jest rozłożony na wiele linii. To jest coś, na co naprawdę muszę spojrzeć, aby zobaczyć, co robi, zamiast być w stanie wziąć to bezpośrednio, widząc tylko parę słów kluczowychelse if
.else if
konstrukcję, zwłaszcza że jej łańcuchowa forma jest tak dobrze znanym wzorem. Jednak kod formularzaif(...) return ...;
jest również dobrze znanym wzorcem, więc nie do końca bym tego potępił. Uważam to jednak za drobny problem: logika przepływu sterowania jest taka sama w obu przypadkach, a jedno bliższe spojrzenie naif(...) { ...; return; }
drabinę powie mi, że rzeczywiście jest równoważneelse if
drabinie. Widzę strukturę pojedynczego terminu, wywnioskuję jego znaczenie, zdaję sobie sprawę, że jest powtarzany wszędzie i wiem, co jest grane.else if
ireturn
. np.else if(...) { return alert();}
Kontrowersyjne jest, czy jest to „lepsze” niż zwykłe, jeśli… w danym przypadku. Ale jeśli chcesz spróbować czegoś innego, jest to powszechny sposób na zrobienie tego.
Umieść swoje warunki w obiektach i umieść te obiekty na liście
Jeśli pod warunkiem jest wymaganych wiele akcji, możesz wykonać szaloną rekursję
Z pewnością tak. Działa to tylko wtedy, gdy masz wzorzec logiki. Jeśli spróbujesz wykonać super ogólną rekursywną akcję warunkową, wówczas konfiguracja obiektu będzie tak skomplikowana jak oryginalna instrukcja if. Będziesz wymyślał swój nowy język / framework.
Ale Twój przykład nie mają wzór
Typowym przypadkiem użycia tego wzorca byłaby walidacja. Zamiast :
Staje się
źródło
if...else
drabinę do konstrukcjiConditions
listy. Zysk netto jest ujemny, ponieważ konstrukcjaConditions
zajmie tyle samo kodu, co kod OP, ale dodatkowa pośrednia obciąża czytelność. Zdecydowanie wolałbym czystą kodowaną drabinę.conditions
jest skonstruowany ... ARG! Nie atrybuty adnotacji! Boże czemu? Ow oczy!Rozważ użycie
return;
po spełnieniu jednego warunku, oszczędza ci to wszystkoelse
. Możesz nawet być w staniereturn addAlert(1)
bezpośrednio, jeśli ta metoda ma wartość zwracaną.źródło
if
s ... To może być rozsądne założenie, a potem może nie być.Czasami widziałem takie konstrukcje uważane za czystsze:
Trójskładnik z odpowiednimi odstępami może być również zgrabną alternatywą:
Sądzę, że możesz także spróbować utworzyć tablicę z parą zawierającą warunek i funkcję i iterować nad nim do momentu spełnienia pierwszego warunku - co, jak widzę, byłoby równe pierwszej odpowiedzi Ewana.
źródło
case
etykietami?Jako wariant odpowiedzi @ Ewan, możesz stworzyć łańcuch (zamiast „płaskiej listy”) takich warunków:
W ten sposób możesz zastosować swoje warunki w określonej kolejności, a infrastruktura (pokazana klasa abstrakcyjna) pomija pozostałe kontrole po spełnieniu pierwszego.
Jest to miejsce, w którym przewyższa podejście oparte na „płaskiej liście”, gdzie musisz zaimplementować „pomijanie” w pętli, która stosuje warunki.
Po prostu konfigurujesz łańcuch warunków:
I rozpocznij ocenę od prostego połączenia:
źródło
Po pierwsze, oryginalny kod nie jest straszny IMO. Jest to całkiem zrozumiałe i nie ma w tym nic złego.
Jeśli więc ci się nie podoba, skorzystaj z pomysłu @ Ewana, by użyć listy, ale usuń nieco nienaturalny
foreach break
wzór:Teraz dostosuj to w wybranym języku, spraw, aby każdy element listy stał się obiektem, krotką, cokolwiek, i jesteś dobry.
EDYCJA: wygląda na to, że nie jest tak jasne, jak myślałem, więc pozwól mi wyjaśnić dalej.
conditions
jest jakąś uporządkowaną listą;head
jest bieżącym badanym elementem - na początku jest to pierwszy element listy i za każdym razem, gdynext()
jest wywoływany, staje się kolejnym;check()
ialert()
tocheckConditionX()
iaddAlert(X)
z PO.źródło
while not
zamiastforeach break
.Pytanie nie ma pewnych szczegółów. Jeśli warunki są następujące:
lub jeśli zawartość
addAlert
jest bardziej skomplikowana, prawdopodobnie lepszym rozwiązaniem, powiedzmy c #, byłoby:Krotki nie są tak piękne w c # <8, ale zostały wybrane dla wygody.
Zalety tej metody, nawet jeśli żadna z powyższych opcji nie ma zastosowania, polega na tym, że struktura jest typowana statycznie. Nie możesz przypadkowo spieprzyć, powiedzmy, pominięcie
else
.źródło
Najlepszym sposobem na zmniejszenie złożoności Cyclomatic w przypadkach, gdy jest ich dużo,
if->then statements
jest użycie słownika lub listy (zależnej od języka) do przechowywania wartości klucza (jeśli wartość instrukcji lub pewnej wartości), a następnie wyniku wartość / funkcja.Na przykład zamiast (C #):
Mogę po prostu
Jeśli używasz bardziej nowoczesnych języków, możesz zapisać więcej logiki niż tylko wartości (c #). To są naprawdę tylko funkcje wbudowane, ale możesz również wskazać inne funkcje, jeśli logika ma zawężać do wbudowania.
źródło
Twój kod jest już za krótki, ale sama logika nie powinna być zmieniana. Na pierwszy rzut oka wygląda na to, że powtarzasz się z czterema wywołaniami
checkCondition()
, a po dokładnym przeczytaniu kodu każde z nich jest inne. Należy dodać prawidłowe formatowanie i nazwy funkcji, np .:Twój kod powinien być czytelny przede wszystkim. Po przeczytaniu kilku książek wuja Boba uważam, że jest to przesłanie, które konsekwentnie próbuje przekazać.
źródło
Załóżmy, że wszystkie funkcje są zaimplementowane w tym samym komponencie, możesz sprawić, że funkcje zachowają pewien stan, aby pozbyć się wielu gałęzi w przepływie.
EG:
checkCondition1()
stałby sięevaluateCondition1()
, na którym sprawdzałby, czy spełniono poprzedni warunek; jeśli tak, to buforuje pewną wartość do odzyskaniagetConditionNumber()
.checkCondition2()
stanie sięevaluateCondition2()
, na którym sprawdzi, czy poprzednie warunki zostały spełnione. Jeśli poprzedni warunek nie został spełniony, sprawdza scenariusz warunku 2, buforując wartość do pobraniagetConditionNumber()
. I tak dalej.EDYTOWAĆ:
Oto, jak należałoby wdrożyć kontrolę drogich warunków, aby to podejście zadziałało.
Dlatego jeśli masz zbyt wiele drogich kontroli do wykonania, a elementy w tym kodzie pozostają prywatne, takie podejście pomaga w utrzymaniu go, umożliwiając w razie potrzeby zmianę kolejności kontroli.
Ta odpowiedź stanowi tylko alternatywną sugestię z innych odpowiedzi i prawdopodobnie nie będzie lepsza niż oryginalny kod, jeśli weźmiemy pod uwagę tylko 4 wiersze kodu. Chociaż nie jest to okropne podejście (i żadne nie utrudnia konserwacji, jak powiedzieli inni), biorąc pod uwagę wspomniany scenariusz (zbyt wiele kontroli, tylko główna funkcja ujawniona jako publiczna, wszystkie funkcje są szczegółami implementacji tej samej klasy).
źródło
anyCondition() != false
.true
zostaną ocenione , PO nie chce oceny warunku 3.anyCondition() != false
w ramach funkcjievaluateConditionXX()
. Jest to możliwe do wdrożenia. Jeśli podejście polegające na użyciu stanu wewnętrznego nie jest pożądane, rozumiem, ale argument, że to nie działa, jest nieważny.Więcej niż dwie klauzule „else” zmusza czytelnika kodu do przejścia całego łańcucha w celu znalezienia interesującego go kodu. Użyj metody, takiej jak: void AlertUponCondition (Warunek) {switch (warunek) {case Condition.Con1: ... break; case Condition.Con2: ... przerwa; itp ...} Gdzie „Warunek” jest właściwym wyliczeniem. W razie potrzeby zwróć wartość bool lub wartość. Nazwij to tak: AlertOnCondition (GetCondition ());
To naprawdę nie może być prostsze, I jest szybsze niż łańcuch if-else, gdy przekroczysz kilka przypadków.
źródło
Nie mogę mówić o twojej konkretnej sytuacji, ponieważ kod nie jest konkretny, ale ...
taki kod często pachnie brakującym modelem OO. Naprawdę masz cztery rodzaje rzeczy, każda związana z własnym typem alertera, ale zamiast rozpoznawać te encje i tworzyć dla nich instancję klasy, traktujesz je jako jedną rzecz i próbujesz nadrobić to później, w czasie musisz wiedzieć, z czym masz do czynienia, aby kontynuować.
Polimorfizm mógł ci bardziej odpowiadać.
Bądź podejrzliwy w stosunku do kodu za pomocą długich metod zawierających długie lub złożone konstrukcje if-then. Często potrzebujesz drzewa klasy z pewnymi wirtualnymi metodami.
źródło