bo trzeba być oryginalnym
Często spotykam się z sytuacją, że programista za wszelką cenę stara się wyeliminować wszystkie powtórzenia w swoim kodzie. Dlaczego? Ponieważ uważa, że duplikacja kodu to czyste zło i każda linijka kodu powinna być unikalna. Tylko czy takie przeświadczenie jest słuszne? Nie.logika i realizacja
Aplikacja, każda, bez wyjątku, składa się z dwóch zasadniczych części - logiki oraz jej realizacji.Czym jest logika? Załóżmy, że mamy stworzyć aplikację do zarządzania restauracją, a dla uproszczenia pełną - obsługą zamówień. Co robimy najpierw? Dowiadujemy się jak to wszystko działa obecnie, czym jest owo zamówienie, z czego się składa, kiedy jest tworzone, a kiedy zamykane, co się dzieje z nim dalej, ile może ich być, kto nimi zarządza, kto z nimi może coś zrobić i co itp., itd. Czyli staramy się pojąć, jak w rzeczywistości ta restauracja obsługuje cały ten proces. I to jest część, która jest niezależna od sposoby realizacji aplikacji, nie ważne w jakim języku ją napiszemy, ba, możemy nawet opisać ten proces w jakimś rzeczywistym dokumencie. Tak czy inaczej nie zmienia się.
Wszystkie informacje zebrane w tym momencie pozwolą nam na opracowanie logiki naszej aplikacji.
Czym jest natomiast owa realizacja? Ma na nią wpływ język, który wybraliśmy, frameworki, ORMy, sposoby zapisu danych itp., itd. Jest to kod, który pozwala nam przekuć wymagania klienta na kod, czyli walidatory, filtry, formularze, widoki itp., czyli jest to pewna funkcjonalność, od której logika jest niezależna. Nawet jeśli jest ona (funkcjonalność) tworzona specjalnie dla danej aplikacji.
przejdźmy to sedna
Po tych krótkich wyjaśnieniach mogę przejść do meritum:)Najczęściej w logice aplikacji nie ma miejsca na duplikację kodu, ponieważ w procesie zazwyczaj to jeden model jest odpowiedzialny za daną rzecz. Dlatego też w modelach obsługujących logikę tych procesów bardzo rzadko jest miejsce na powtórzenia kodu. Więc, jeżeli posiadacie te same fragmenty kodu w różnych miejscach, to warto się nad nimi poważnie zastanowić. Albo wynieść je wyżej, albo przeanalizować jeszcze raz rozwiązanie, na które się zdecydowaliście. Warto poczytać trochę na temat wysokiej spójności i niskiem sprzężeniu, które poruszają to zagadnienie.
Jednak inaczej rzecz się ma z funkcjonalnością. Nie zawsze unikanie powtarzającego się kodu jest dobrym pomysłem. Dlaczego? Często (a na pewno dużo częściej niżbyśmy chcieli) prowadzi to do utworzenia drzew dziedziczeń. Mamy kilka klas z bardzo ubogą funkcjonalnością, po których dziedziczy cała masa innych. Tylko dlatego, żeby uniknąć trzech-czterech linijek kodu. Najgorsze jednak są sytuacje, gdy klasy pochodne nie są ze sobą w żaden sposób powiązane. Rodzą się zależności, które nie powinny mieć miejsca i pewnego dnia odkrywamy, że aby wprowadzić modyfikację do widoku trzeba przepisywać filtry. Brzmi absurdalnie? Może, ale niestety takie sytuacje mają miejsce.
na koniec
Zdaję sobie sprawę, że może trochę wyolbrzymiłem cały problem, jednak miało to na celu tym wyraźniejsze zwrócenie Waszej uwagi na problem.Zgadzam się, że kod zduplikowany należy usuwać, refaktoryzować klasy tak, aby było go jak najmniej, jednak należy to robić ostrożnie, ponieważ czasami może się zdarzyć, że w przyszłości przyniesie to więcej szkód niż pożytku.
przykład
[edit 2012-06-04]W związku z tym, że w prawie każdym komentarzu jest prośba o przykład o to i on:)
Wyobraźcie sobie, że mamy aplikację z dziesiątkami kontrolerów (co wcale nie jest trudne do osiągnięcia:). Oczywiście taka ilość kontrolerów spowodowała, że wydzieliliśmy kilka klas abstrakcyjnych, ponieważ jest kilka grup kontrolerów, których funkcjonalność jest bardzo podobna. Całkiem prawdopodobny scenariusz? Wydaje mi się, że tak:)
W aplikacji mamy możliwość edycji swoich danych, personalizowania jej ustawień i kilka innych rzeczy ustawianych per user. Ponieważ i zarządzanie danymi i personalizowanie ustawień są na tyle złożonymi przypadkami użycia i w dodatku nie atomowymi (czyli do obsługi każdego jest kilka metod) decydujemy się na stworzenie dwóch kontrolerów na tą okazję. Brzmi rozsądnie? Mam nadzieję:)
Podczas edycji/personalizacji do pobierania użytkownika, którego dane mają być modyfikowane, wykorzystujemy metodę z klasy nadrzędnej getUser(), która zwraca obiekt będący reprezentacją aktualnie zalogowanego użytkownika.
Okazuje się jednak, że tymi danymi (większością) może zarządzać administrator. Prawdopodobe?
Na szczęście kontrolery do zarządzania użytkownikami już mamy, pozostaje jedynie rozszerzyć ich funkcjonalność. Decydujemy się na nadpisanie metody getUser(). Zwraca ona aktualnie zalogowanego użytkownika lub użytkownika, który został zdefiniowany w żądaniu np. poprzez podanie id. W ACL'u sprawdzamy (w przypadku przesłania id), czy użytkownik to admin, czy nie.
Działa? Nawet całkiem nieźle:) Ale co dalej? Przecież mamy dwa kontrolery tego typu! W dwóch, w identyczny sposób została nadpisana metoda getUser()! Tworzymy klasę abstrakcyjną!
Oczywiście takich metod, których implementacja to dwie, trzy linie kodu, jest mnóstwo, czasami się powtarzają. Czy na każdy przypadek mamy tworzyć klasę abstrakcyjną? A co, gdyby jedna z klas z przykładu zawierała również kilka metod, które znajdują się w innych klasach? W rozbudowanych aplikacjach taki scenariusz wcale nie jest nieprawdopodobny.
Rozwiązań problemu jest kilka:
- tworzenie klas abstrakcyjnych
- skorzystanie z helperów lub innego rodzaju funkcjonalności, która jest wywoływana w magiczny sposób
- pozostawienie duplikatów w kodzie
Jak ze wszystkim w życiu trzeba znać umiar, trudno się nie zgodzić co do treści posta pewne nawyki w programowaniu trzeba sobie po prostu wyrobić.
OdpowiedzUsuńSamo powielenie kodu nie jest niczym złym ale trzeba to robić z umiarem i rozwagą bo jeżeli przesadzimy w drugą stronę to znów otrzymamy małego potworka z naszego kodu :)
Brakuje mi tutaj tylko praktycznego przykładu kodu z takimi powtórzeniami.
Często właśnie pakuję się w unikanie dublowania kodu i często kończy się to jak w artykule - rozbudowane dziedziczenie itd. Ostatnio zaś programowałem sporo stosując stare, dobre programowanie funkcyjne (C itp.) - problem dublowania znów się pojawił...
OdpowiedzUsuńWalka z dublowaniem prowadzona w trybie "za wszelką cenę" jest w którymś momencie sztuką dla sztuki.
Jeśli chodzi o przykład, o którym wspomniał kolega we wcześniejszym komentarzu, myślę, że możnaby tu zestawić dowolny fragment kodu z i bez eliminacji i porównać liczbę wierszy i poziom kompilkacji, a dalej - jeśli dalibyśmy radę - to, co z kodem zrobi kompilator.
Jak tylko znajdę chwilę, to dodam jakiś przykład:) Postaram się prędzej niż później:P
OdpowiedzUsuńCiekawy artykuł.
OdpowiedzUsuńW językach typu Ruby zamiast dziedziczenia można stosować mixiny, które można używać statycznie (include do klasy), albo dynamicznie (rozszerzenie obiektu). To pozwala na reużycie, jednak bez wad, które wymieniłeś.
Polecam też poczytanie o architekturze DCI, która próbuje rozwiązać dokładnie ten problem.
Podaj przykład, bardzo proszę, bo póki co kompletnie nie mogę się zgodzić. Duplikacja kodu to jest czyste zło - z różnych powodów. Należy z nią usilnie walczyć.
OdpowiedzUsuńA dlaczego ta walka ma prowadzić do nieprawidłowych drzew dziedziczeń? Nie wiem. Nie chcesz chyba powiedzieć, że za jedyny sposób na wielokrotne wykorzystanie kodu uważasz dziedziczenie.
"Duplikacja kodu to jest czyste zło - z różnych powodów"
UsuńPopadanie w skrajności to czyste zło. Jednak nadmieniłeś o 'różnych powodach', więc czy mógłbyś rozwinąć temat?
Nie uważam, że wielokrotne wykorzystanie kodu sprowadza się do dziedziczenia, ale usuwanie linijek kodu tylko dlatego, że 'już je gdzieś widziałem' najczęściej 'owocuje' właśnie hierarchią klas, którą nie da się w prosty sposób zarządzać.
Niekoniecznie. Dziedziczenie to najmocniejszy związek między klasami i nie można go nadużywać, bo robią nam się duże zależności i później, jak wspomniałeś, ciężko tym zarządzać, utrzymywać i rozwijać. Rozbudowane drzewa dziedziczenia nie są oznaką dobrze zaprojektowanej aplikacji.
UsuńDlatego kompozycja jest lepszym rozwiązaniem w wielu wypadkach. Jeśli mamy jakiś kod który się powtarza w wielu miejscach, to może nie twórzmy nadklasy aby tylko upchać tą metodę, ale tą i inne metody o podobnych odpowiedzialnościach wydzielmy do całkowicie osobnej klasy, której obiekty wstrzykniemy tam gdzie tego potrzebujemy.
Umieszczając kompozycje w Twoim przykładzie, klasa kontrolera dla admina i usera powinna być jedna, a obiekty tych kontrolerów mogą być parametryzowane obiektem klasy np. UserProvider, która posiada metodę getUser. Jak jest pobierany użytkownik, z sesji, czy na podstawie parametrów żądania, to powinno kontroler mało obchodzić ;)
Zgadzam się, że kompozycja jest rozwiązaniem problemu, ale tylko pod warunkiem, że mamy na myśli jakąś złożoną/istotną funkcjonalność lub rzeczywiście chodzi o kilka metod.
UsuńAle jeżeli mamy metodę (w dwóch odrębnych logicznie klasach):
private function _getSomething()
{
if (is_null($this->_something))
$this->_something= new Something();
return $this->_something;
}
jest ona na tyle mało istotna, że chyba nic się nie stanie, jeżeli będzie znajdowała się w dwóch klasach:)
Ten przykład to paskudne powielanie kodu i uzależnienie się od konkretnej implementacji. Leniwa inicjalizacja jest dobra. Źle się dzieje jeśli przez to uzależniamy dwie całkowicie odrębne klasy od konkretnej, tej samej implementacji. To zalążek do tego, aby kod się w niedalekiej przyszłości psuł i śmierdział... Nie chociaż taki kod śmierdzi od czasu jego napisania ;)
UsuńW tym przypadku mamy dwa obiekty tej samej klasy, a wcale dwóch nie potrzebujemy, wystarczy jeden (chociaż to może zależeć od konkretnego przypadku). Do takich rzeczy służy wstrzykiwanie zależności z zewnątrz (nie mówię tu nawet o kontenerze wstrzykiwania zależności, to inna bajka). W Twoim przypadku dwie klasy z metodą _getSomething() wiedzą jak budować dany obiekt, a co będzie jeśli zmieni się sygnatura budowanego obiektu? Trzeba będzie zmieniać tak samo dwa kawałki kodu, nie trudno zapomnieć o jednym z nich. PHP to język interpretowany, kompilator nam nie podpowie że coś jest nie tak, wywali się na produkcji i może być problem ;)
Powielanie kodu jest bardzo złe, ale istnieją nieliczne wyjątki w których jest "przydatne". Jednym z takich przypadków jest implementacji obiektów transferu danych np. w usługach sieciowych. Są one (przynajmniej początkowo) bardzo podobne do modelu dziedziny, ale nie powinny być od niego całkowicie zależne, gdyż zewnętrzne api powinno być odporne na zmianę interfejsu i implementacji modelu dziedziny. Wszystkie przykłady które podajesz są wg mnie nietrafione.
Nawet nie wspomnę o ograniczeniu testowalności w przypadku gdy zaszyjesz na sztywno zależności wewnątrz klas, ale to jest osobny temat ;)
Tak z ciekawości, to jak byś rozwiązał ten problem?
UsuńZrobić wstrzyknięcie tego obiektu? Jeżeli tak to dlaczego, skoro to obiekt tworzący go powinien być za to tworzenie odpowiedzialny? W tym wypadku dwa obiekty.
Dziedziczenie? Raczej nie, bo jeżeli klasy dostarczają różnej funkcjonalności, to tworzenie nad nimi czegoś wspólnego nie jest dobrym pomysłem.
Co do testowanie, to jeżeli wszystko jest przetestowane, to ewentualne zmiany w kodzie (które będą modyfikowały api klasy) szybko zostaną zauważone.
zastosuj kompozycje zamiast dziedziczenia ktore jest zueeem i problem z drzewami dziedziczeń zniknie, bedziesz sobie dobierał komponenty wedle woli.
OdpowiedzUsuńpowtórzenia to złoo