Wzorzec projektowy, unikanie if-ów, dobre praktyki

Witam serdecznie.

Mam taki kłopot nad zastosowaniem odpowiedniego wzorca do rozwiązania takiego problemu:

zrobCos() {
    if(warunek1){
        zrob1();
    }else 
	if(warunek2){
        zrob2();
    }
}

Moje podejście do tego typu sytuacji jest mniej więcej takie:

Tworzę interfejs

Interface ZrobCos 
    zrob();

KlasaWykonawca {
    metodaX(ZrobCos zrobCos) {
        zrobCos.zrob();
    }

}

Oraz obiekty implementujące ten interfejs , które odpowiednio wykonują zadane operacje według przeznaczenia .
Oczywiście to okrojony przykład

Problem polega na tym, że z front-endu idzie parametr, który określa która implementacja ma być użyta co prowadzi do użycia if-ów lub case.

Moim pomysłem jest przekazanie parametru określającego nazwę klasy jako parametru przychodzącego z front-endu. Przy pomocy refleksji tworzymy instancję tej klasy na podstawie przekazanego parametru. Pozbywamy się if-ów.

Czy moje rozwiązanie jest akceptowalne ? Ma ktoś ciekawsze podejścia ?
Może jakieś inne dobre praktyki eliminacji if-ów.

Pozdrawiam

http://koziolekweb.pl/2011/10/26/ekstremalna-obiektowosc-w-praktyce-%E2%80%93-czesc-2-nie-uzywaj-slowa-kluczowego-else/

poza tym parametr z nazwą klasy to super fajna podatność na ataki.

Tak masz rację, dzięki za artykuł.

To jakieś wariactwo. Czego jeszcze niby to mamy unikać?

„złożoność cyklomatyczna”? absurdalna miara, i kto powiedział, że powinna być jak najmniejsza?

Im kod mniej złożony wg. tej miary tym łatwiej jest go utrzymywać. Nie masz sytuacji gdzie z jednej metody masz n punktów wyjścia. Zobacz, że na forum często trafiają się kwiatki z drabinkami ifów gdzie jedyne co się nasuwa na myśl to "o rzesz ty w mordę".
Metoda mocno z dużą ilością ifów zaczyna robić wiele rzeczy i tym samym traci sens istnienia.

To nie ma znaczenia. Drabinkę ifów zamieniamy na switch, ale to tylko lukier składniowy, bo nadal jest tym samym. Autor artykułu sugeruje zamianę if-else na polimorfizm.

W jaki sposób zmniejsza to złożoność jakąś-tam programu? Może według jego świętego graala, złożoności cyklo-cośtam, tak mu wychodzi; ale jak się zastanowić, to:

• Dany fragment programu ma mieć dokładnie pierdyliard możliwych przebiegów. Nie wolno nam tego „uprościć”, bo łamalibyśmy specyfikację.

Nie ma znaczenia jak to zapiszemy: czy w jednej wielkiej funkcji, czy w tysiącu jednolinijkowców. Nie ma znaczenia czy użyjemy if-else, czy miliona funkcji wirtualnych symulujących if-else. Nadal program (funkcja, moduł) musi mieć dokładnie ten pierdyliard możliwych przebiegów.

Dlatego nie ma sensu liczenie dziwnych metryk, które nas doprowadzają do absurdalnych wniosków. Kod powinien być prosty, czytelny. A prostsze, czytelniejsze, i szybsze w pisaniu oraz działaniu, jest zwykłe if-else.

Zgadzam się.
Im niej złożony kod tym łatwiej go utrzymać. Przejrzystość sama nasuwa rozwiązania.

Problem w tym, że ja <ort>od kąt</ort> zajmuję się it to nie natrafiłem na projekt, który był napisany zgodnie z paradygmatami programowania obiektowego.
Większość klas posiada <ort>nie poprawne</ort> nazewnictwo. Nie wspomnę o tym, że wykonują prace, które nie są im przypisane. Błędne nazewnictwo metod. Można by tak wypisywać bez końca . Jak się poruszać w tym.
Dlatego jestem zwolennikiem unikania if-ów, zasad GRASP, SOLID. Tylko jak je zastosować w projekcie, który jest źle zaprojektowany od samego początku.

"A prostsze, czytelniejsze, i szybsze w pisaniu oraz działaniu, jest zwykłe if-else."

Chyba dla kogoś kto od zawsze pisał wszystko w if-else , i pracował z językami proceduralnymi.

@Azarien, można też cały program zamknąć w jednej klasie. Summa summarum wyjdzie na to samo, dowalimy jeszcze tylko kilka GOTO i będzie super. Jeżeli sądzisz, że zwykłe "if-else" jest proste w rozumieniu to najwidoczniej nigdy nie widziałeś poprawnie napisanego programu np. z użyciem wzorca strategii. Po to ktoś znacznie mądrzejszy od nas wykombinował sobie, że znacznie wygodniej jest pozbyć się IFów i w ich miejsce wstawić jakieś rozwiązanie na poziomie architektury byśmy mogli używać tego jego "wynalazku" i nie męczyć się z klasami potworkami na 10k linii.

I jeszcze polecam z zeszłorocznej Confitury http://parleys.com/#st=5&id=3372&sl=0 (wersja ze slajdami)

@Azarien ja się zgadzam tutaj z @Koziołek. Miałem okazję widzieć metodę która miała 600 (!) linii i była złożona z samych if'ów a maksymalny poziom zagłębienia wynosił 10. I wszystko przez to że ktoś zaczął ją w ten sposób pisać, a potem kolejne osoby uznawały że musiałyby to refaktorować długi czas albo dopisać jednego ifa po prostu ;]
Nie muszę chyba mówić ile było defektów do tej metody ;)
Jasne że usuwanie tego przez polimorfizm zrobiłoby pewnie +100 klas przynajmniej, ale to nie problem. Bo dzięki temu, gdyby coś nie działało, wiadomo byłoby gdzie szukać problemu i wystarczyłoby poprawić klasę która ma 20 linijek, a nie kluczyć po kolosalnej drabince ifów.

Ja nie jestem jakimś ultra purystą. Robienie takich cudów jak się ma jeden if..else jest bez sensu, ale jak sie ich ma 4 czy 5? Bo potem dopisanie 6,7,8 jest znacznie szybsze niż refaktorowanie i nagle się okazuje że ma się ich 100 ;)

Zastępowanie każdego if-a polimorfizmem to jakiś ideologiczny idiotyzm. Przede wszystkim powinno się pisać kod w sposób pragmatyczny, tak by można było w przyszłości do tego szybko przysiąść nie pamiętając (czy wiedząc, jeśli ktoś inny pisał) i zmodyfikować. Nie ma uniwersalnej zasady określającej kiedy stosować jaką konstrukcję to raczej kwestia doświadczenia i tego czy się myśli czy tylko bezmyślnie klepie aby działało.

Hmm....to spójrzcie na ten przykład:
http://sharpek.net/blog/2009/08/07/wzorce-projektowe-strategia/

Całkiem fajne wytłumaczenie wzorca projektowego Strategia, ale niech ktoś powie z ręką na sercu że z ifami jest mniej czytelne niż po zastosowaniu wzorca:)

Myślę, że wszędzie jest potrzebny złoty środek - metoda 600 linii czy nawet 60 to za dużo, ale znowu napłodzenie obiektów tylko po to by być "poprawnym" to też skrajność. Ktoś wspomniał tutaj, że kod nie musi być "wzorcowy", ale na pewno pragmatyczny i chyba każdy kto analizował nie swój kod choc trochę bardziej złożony niż hello world to przyzna.

Zależy od ifów, ale często gęsto wygląda to tak:

if(...){
   logika 1
}
else if(...){
   logika 2
}
else if(...){
   logika 3
}
...
else if(...){
   logika 99
}

Gdzie przez logikę rozumiemy coś więcej niż delegację zadania do jakiejś klasy.

1. w kodzie z artykułu są drobne nieścisłości (nie wiem co to za język - Java?):

Chyba powinno być coś na początku funkcji co zamienia object na service.

public AbstractServiceStub(Object object) {
		if (service == null) {
			super.service = new Service();
		} else {
			super.service = service;
		}
		((Service) super.service).setTypeMappingVersion("1.1");
	}

1. Zamiast kodu:

private void initService(final Service service) {
		if (Assertions.isNull(service)) {
			super.service = new Service();
			return;
		}
		super.service = service;
	}

ładniej napisać:

private void initService(final Service service) {
		if (Assertions.isNull(service)) {
		  super.service = new Service();
		} else {
		  super.service = service;
               }
	}

1. rezygnacja z "else" to wariactwo, z artykułu nie wynika że tylko w konkretnych przypadkach uzasadnione.

Przecież if-else jest podstawową konstrukcją wszystkich języków. Zamienianie ich na poliformizm w przypadku:

if (warunek){
   policzA();
} else {
   policzB();
}

, to jakaś nędzna próba pokazania "ale ja jestem fajny, popatrzcie, jakie znam ekstra sztuczki".

@kaskader_kl, a popatrz na to inaczej. Pozbywając się if-else ze swojego kodu ograniczasz ilość rozgałęzień i tym samym np. łatwiej taki kod przetestować.

chyba najprostszym sposobem do zaprzyjaźnienia się z wymianą ifów-elsów na polimorfizm to pisanie parserów ( co czasami się pojawia ), albo systemu zarządzania statusami np. dokumentów, gdzie jeszcze każda zmiana statusu trigeruje jakąś akcję :D konstrukcje z ifami powodują od razu odruch wymiotny.

W porządku - tam gdzie należałoby zrobić drzewko if-ów, rozgałęziając program na 10 sposobów, polimorfizm jest dobrym rozwiązaniem. Wciskanie go na siłę do jednego if-a, to przerost formy nad treścią. @Koziołek mówi, że to ograniczenie rozgałęzień, tyle tylko, że to wciąż jest rozgałęzienie (albo wykona się metoda x() z klasy A, albo z klasy B), tylko ukryte, więc trudniejsze to testów/debugowania.
Dążę do tego, że w przypadku prostych if-ów, skomplikowane konstrukcje zmniejszają czytelność kodu.

kaskader_kl napisał(a):

@Koziołek mówi, że to ograniczenie rozgałęzień, tyle tylko, że to wciąż jest rozgałęzienie (albo wykona się metoda x() z klasy A, albo z klasy B), tylko ukryte, więc trudniejsze to testów/debugowania.

Chyba żartujesz. Zastanów się nad tym jeszcze raz.

Ależ zastanowiłem się. Chodzi nam o to, żeby program robił różne rzeczy w zależności od stanu (w tym wartościowania zmiennych), co w prostych przypadkach robi się prostym if-em. Jeśli zastosujemy polimorfizm, to osiągamy ten sam rezultat, więc wciąż jest to rozgałęzienie.
Spójrz na taką sytuację:
W zależności od parametrów uruchomieniowych serwera (podanych w pliku czy przy uruchomieniu) program ma obsługiwać zarządzanie użytkownikami na własnej bazie danych lub z użyciem Kerberosa. Zrobienie jednej klasy z metodami checkUser(), addUser() itd, a w każdej z nich if(warunek){kerberos} else {baza}, to poroniony pomysł.
Tutaj w sukurs przychodzi nam polimorfizm i nie mogę się z tym nie zgodzić.
Jeśli jednak chcemy sprawdzić, czy obiekt spełnia warunki dodawania do obszaru roboczego, to wtedy logiczne, wygodne i oczywiste jest zrobienie if (warunek(obiekt)){wstaw(obiekt);} else {wyrzuc(obiekt)}. Po cholerę tutaj coś komplikować?

A ja pomału podejrzewam, że to chodzi o ideeologię:P
Może jest tak, że inż, którzy mieli nie tylko styczność z OOP, ale również np z programowaniem sterowników- drabinkami itp. sprawniej postrzegają zmiany stanów jako coś naturalnego. Łatwiej im testować/debugować kod, w którym są "wejścia/wyjścia" i "siatka stanów".
Programości po informatyce (niekiedy też inż. ale jednak innego typu:P) od A do Z przynajmniej na studiach uczą się tak naprawdę jednego rodzaju myślenia-jak najbardziej poprawnego i nawet profesjonalnego tak że na początku pracy mają łatwiej niż Ci pierwsi.
Wnioskuje to po kilkuletniej pracy w korpo, gdzie widzialem kod dośw prog/arch, którzy jednak wywwodzili się z różnych "początków".

Trudniejsze do debuggowania?

1. Masz błąd - stacktrace
2. W stacktrace masz wymienioną klasę - przez nazwę wraz z numerem linii.
3. Możesz się zapiąć debuggerem w konkretnym, krótkim, pliku, a nie szukać linii w 300 linijkowym ifie.

W dodatku np. Eclipse czasami nie może dogadać się z debuggerem w temacie "w której linii dokładnie jesteśmy" i wychodzą fajne jaja.

Co do przykładu ze wstawianiem obiektu, to w moim wpisie chodzi o to by w pierwszej kolejności należy unikać słówka kluczowego else, które potrafi zagmatwać kod (dodatkowe 1-3 linii). Poza tym podany przez ciebie przykład też można "odifować" i nawet opłaca się to robić jeżeli np. chcesz mieć możliwość wstawiania/odrzucania obiektu w specyficzny sposób.

Oczywiście, że można, tylko po co? Dążę do tego, że w prostych przykładach wystarczy if(warunek){metoda}else{metoda2} i nie wyobrażam sobie nic szybszego i czytelniejszego.
Tworzenie do tego nowych klas, to więcej, niż dodatkowe 1-3 linii, a takie 1-3 linie nie tworzą od razu 300-linijkowych ifów.

kaskader_kl napisał(a):

Jeśli jednak chcemy sprawdzić, czy obiekt spełnia warunki dodawania do obszaru roboczego, to wtedy logiczne, wygodne i oczywiste jest zrobienie if (warunek(obiekt)){wstaw(obiekt);} else {wyrzuc(obiekt)}. Po cholerę tutaj coś komplikować?

Ale taki kod i tak w 99% można POPRAWIĆ i nie komplikować go, no chyba że to cały kod w funkcji. :)

OK. Piszę sobie taki projekt:
Tworzę bryłę 3D i w każdym punkcie chcę wstawiać małe sześciany. Niestety muszę sprawdzać, czy mogę wstawić dany sześcian i robię to zewnętrzną aplikacją (skomplikowana rzecz, nie warta przepisania), z którą rozmawiam po TCP.
Iteruję sobie zgodnie z jakąś heurystyką po obszarze roboczym i dla każdego punktu robię:

if (sprawdz(punkt)){
  wstaw(punkt);
} else {
  zaznaczJakoOdrzucony(punkt);
}

Nie za bardzo widzę, jak to można poprawić/uprościć/ułatwić debuggowanie lub czytanie.

@kaskader_kl: nie wiem po co się wysilasz udowadniając że IF-y mają zwykle sens.
To raczej opozycjoniści (w tym @Koziołek) napisali coś co sprawdza się tylko w pewnych przypadkach i to oni powinni wskazać kiedy to ma sens.
Ale albo tego nie wiedzą, albo wolą szokować siejąc tanią sensację.

Wzorzec strategii jest bardzo popularny i wiedza o nim jest łatwa do zdobycia (np. kiedy go stosować), dlatego nie rozumiem po co jeszcze dalej to ciągniecie.
http://userpages.umbc.edu/~tarr/dp/lectures/StateStrategy.pdf
http://sourcemaking.com/design_patterns/strategy
http://www.oodesign.com/strategy-pattern.html
http://java.dzone.com/articles/design-patterns-strategy

Przykład kiedy strategia ma sens:
http://blogs.microsoft.co.il/blogs/gilf/archive/2009/11/22/applying-strategy-pattern-instead-of-using-switch-statements.aspx

Można wywalić else:

boolean wstawJakMozna(Punkt punkt){
  if (sprawdz(punkt))
    return wstaw(punkt);
  return zaznaczJakoOdrzucony(punkt);
}

i już jest, prościej.

Głównym celem przedstawionym w tekście jest eliminacja słowa kluczowego else/dużej ilości ifów. Nigdzie nie wspominałem o całkowitej eliminacji ifów. Podałem jednak pewne rozwiązania, które moim zdaniem, są lepsze niż drabinki - polimorfizm jest jednym z nich. Inne to delegacja do biblioteki zewnętrznej - wzorzec strategii, programowanie aspektowe, czy też w przypadku if ==null użycie NullObject.

swoją drogą wersja bez ifowa w scali:

var punkty = List(...)
var ok = punkty.filter(x => sprawdz(x))
ok.foreach(x => wstaw(x))
(punkty -- ok ).foreach(x => zaznaczJakoOdrzucony(x))

a jak znam życie da się prościej

Ciągle powtarza się argument o "drabinkach", metodach na 400 linii i podobnych patologicznych przypadkach.

A ja twierdzę, że w takich prostych warunkach:

if (x > 100)
{
    warning("za dużo, obcinam do 100...");
    x = 100;
}

if (trzebaZrobićCośtam())
    zróbCośTam();

if (godObject == null)
    error("nie poleci");

if (godObject == null)
    godObject = new godObject();

wymyślanie czegokolwiek innego niż if to jest właśnie patologia.

Wzorce i polimorfizm mają swoje miejsce, proste warunki mają swoje miejsce.

Koziołek napisał(a):

Można wywalić else:

boolean wstawJakMozna(Punkt punkt){
  if (sprawdz(punkt))
    return wstaw(punkt);
  return zaznaczJakoOdrzucony(punkt);
}

i już jest, prościej.

Nie, nie jest prościej.
Usunąłeś jedno słowo, które nie ma (w tym przypadku) wpływu na semantykę kodu, ale ma wpływ na czytelność.

boolean wstawJakMozna(Punkt punkt){
  if (sprawdz(punkt))
    return wstaw(punkt);
  else
    return zaznaczJakoOdrzucony(punkt);
}

@Azarien i już masz babola w tym kodzie.

co do scali:

val  p = List(...)
(p -- p.filter(x => sprawdz(x)).map(x => {wstaw(x); x;})).foreach(x => zaznaczJakoOdrzucony(x))