Jak refaktorować – 9 porad, które pomogą naprawić Twój kod
Wykonanie dużego refaktoringu, nie jest zadaniem prostym. Wymaga skupienia, doświadczenia i dyscypliny. Robiłem to wielokrotnie, z różnymi rezultatami. Z każdym następnym razem staję się coraz bardziej doświadczony. W trakcie swojej pracy udało mi się wypracować kilka zasad, które czynią tę czynność łatwiejszą i bezpieczniejszą. Część z nich to dobre praktyki przy refaktoringu, a cześć to po prostu zasady tworzenia dobrego kodu, które w tej sytuacji pomagają mi ogarnąć kod. Wszystkie jednak odpowiadają na pytanie: „Jak refaktorować?”. Zapraszam serdecznie!
1. Zrozum
Zaczynając refaktoring zawsze zaczynam od czegoś innego niż rzucenie się w wir kodowania. Z doświadczenia wiem, że refaktoring jest bardziej skomplikowany, niż się na początku wydaje. W związku z tym, jeszcze przed zmianą pierwszej linii kodu, powinieneś odpowiedzieć sobie na trzy ważne pytania.
Co ten program robi?
Jest to najbardziej podstawowa rzecz, którą musisz wiedzieć. Prawdopodobnie masz wrażenie, że jest to oczywiste. Nie mam na myśli jednak informacji, że jest to np. aplikacja do śledzenia postępu na siłce, sklep internetowy, czy aplikacja intranetowa. Bo to informacja zbyt ogólna. Potrzebujesz raczej informacji jak użytkownicy z niej korzystają, co jest dla nich ważne. Skąd dane są pobierane, gdzie się je zapisuje i dokąd wysyła. Dzięki temu będziesz w stanie zrozumieć perspektywę użytkownika.
Co podmiot programistyczny miał na myśli?
Gdy już wiesz co dana aplikacja robi, potrzebujesz zajrzeć w kod. Zobacz co tam jest, ale jeszcze nic nie zmieniaj. Przedebuguj się przez główne scenariusze i zwróć szczególną uwagę na architekturę, komunikację pomiędzy warstwami i komponentami. Przeanalizuj fragmenty gdzie kod jest najbardziej skomplikowany i najmniej czytelny. Nie próbuj myśleć jak rozplątać to przepyszne spaghetti, jeszcze nie teraz. Na razie spróbuj odgadnąć myśli poprzedniego programisty i odpowiedzieć sobie na pytanie. Dlaczego zostało to napisane w taki sposób, gdzie programista natknął się na problem, a gdzie poszedł na skróty.
Co chcesz osiągnąć?
Kiedy już myślisz, że wiesz co dany kod robi, gdzie są jego najbrudniejsze kawałki oraz z jakimi problemami mógł borykać się jego twórca, możesz wreszcie przejść do pierwszej części pracy twórczej. Podczas tego etapu lubię odejść od komputera, najchętniej na pole, zaczerpnąć tlenu i usiąść w spokojnym miejscu. Kiedy już mam dobre warunki rysuję jak wygląda aktualna architektura. Dzięki temu mogę dostrzec, jak wyglądają przepływy, gdzie są nieścisłości, a gdzie przebiega główna linia programu. Mogę wtedy ocenić co jest nie tak i co należałoby zmienić. Dzięki takiemu zabiegowi mogę wyznaczyć sobie cel jaki chcę osiągnąć i wstępny plan realizacji. Jednak pamiętaj nawet najlepszy plan będzie wymagać modyfikacji.
2. Częste commity
Skoro minął rok 2005 i wiele wspaniałych technologii jest w zasięgu ręki to nie bójmy się z nich korzystać. Zwłaszcza że część z nich może oszczędzić masę czasu, nerwów i pieniędzy. Przykładem takiego narzędzia jest system kontroli wersji git, pozwalający, nad wyraz łatwo, tworzyć lokalne branche i commity. Od kiedy nauczyłem się go używać moja praca stała się dużo przyjemniejsza i wydajniejsza. Twoja również taka może być. Jak? Opisuje i objaśniam.
Commituj często, nawet bardzo często. Jak często? Straszliwie często!!! Dlaczego? Jak robisz jedną dobrą zmianę to wrzuć ją do repozytorium i o niej zapomnij. Niech Twoja głowa patrzy w przód, a nie w tył. W sytuacji, gdy napsujesz coś w kodzie albo zabrniesz w ciemną uliczkę zer i jedynek, zawsze będziesz mógł wrócić do stabilnej wersji sprzed kilku, czy kilkunastu minut, a nie godzin, czy co gorsza dni.
Nie miej obaw, że zaśmiecasz repozytorium, bo tak nie jest. Osobiście jestem w stanie wyprodukować kilkadziesiąt commitów na godzinę. Dlaczego tak dużo? Bo jak coś zmienię lub napiszę coś nowego, co mogę opisać jednym zdaniem, to komituję. Przykładowo: przenoszę plik – commit – zmieniam nazwę pliku – commit – zmieniam nazwę klasy – commit – aktualizuję namespace – commit – zmieniam nazwę zmiennej – commit – … i tak dalej i tak dalej. I czy to wygląda źle? Wręcz przeciwnie, taka historia w repozytorium jest naprawdę wartościowa, ponieważ przechowuje o wiele więcej niż tylko zmiany na kodzie. Na jej podstawie można prześledzić całą ścieżkę zmian i poznać tok rozumowania autora. Oczywiście, jeżeli komity są dobrze nazwane i widać, że leżały na innym branczu, ale to już inna historia.
3. IHaveNoIdeaWhatItIsException
Refaktorowany kod jest często skomplikowany. Niektóre fragmenty aplikacji, mogą być wręcz praktycznie niemożliwe do zrozumienia na początkowym etapie analizy. Na takie sytuacje wypracowałem sobie dość prostą metodę. Zostaw ten kod na później, do momentu, aż Twoja wiedza domenowa i techniczna wzrośnie. Jak odznaczyć kod na później i mieć pewność, że w przyszłości o nim nie zapomnisz? Jest prosta sztuczka. Kiedyś stosowałem znaczniki //TOTO: … Rzeczywistość jednak mocno mnie kopnęła, ponieważ o takim znaczniku bardzo łatwo zapomnieć. Zacząłem i polecam Ci rzucać własny wyjątek IHaveNoIdeaWhatItIsException.
Dzięki temu prostemu zabiegowi nie zapomnisz o tym miejscu, które zostawiłeś na później. Na pewno wrócisz do niego w przyszłości, ponieważ albo gdzieś zostanie wyrzucony, albo na sam koniec refaktoringu będziesz chciał usunąć ten wyjątek. W jednej jak i w drugiej sytuacji, będziesz już lepiej rozumiał kontekst. Kod w okolicy będzie już wyczyszczony, a ten fragment, w otoczeniu nowego, czystego kodu, będzie łatwiejszy do zrozumienia i poprawienia.
4. Szybki return
Jest to zasada, która stosuję prawie zawsze. Według mnie, kod szybko zwracający jakieś wartości jest dużo łatwiejszy do zrozumienia niż zagnieżdżone ify. Ponieważ, przy tak napisanym kodzie złożoność cyklomatyczna drastycznie spada.
Zamiast czegoś takigo:
public bool ValidateCode(string code) { if(code != null) { if(code.Length == 7) { if(IsFormatCorrect(code)) { return true; } } } return false; }
preferuję coś takiego:
public bool ValidateCode(string code) { if(code == null) { return false; } if(code.Length != 7) { retrun false; } if(IsFormatCorrect(code) == false) { return false; } return true; }
5. Klasosranie
Ta metoda przydaje się bardzo w sytuacji, gdy napotkasz mega-hiper-uber klasę. Gdy tylko pomyślę o takim potworze, odzywają mi się zakwasy na palcach od skrolowania. Dodatkowo jestem pewien, że nie będę miał pojęcia co jest w środku. To przerasta mój mózg. Zatem jak sobie z tym porazić, gdy nie bardzo wiadomo z której strony, można to ugryźć, aby nie popsuć. Zwłaszcza, w sytuacji, gdy taka jest używana praktycznie każdym zakamarku aplikacji.
Zatem co proponuję. Znajdź najmnijeszą/najprostrzą metodę z tej klasy. Powiedzmy, niech się nazywa GetProductByCode. Wytnij ją. Stwórz nowy plik. Zadeklaruj w nim klasę ProductByCodeGetter i wklej tę funkcję do niej. Spróbuj skompilować. W tym momencie dostaniesz błędy, ponieważ ta metoda była gdzieś używana. W tamto miejsce wstaw:
var productByCodeGetter = new ProductByCodeGetter(); var product = productByCodeGetter.GetProductByCode(code);
Dzięki takiemu zabiegowi Twój kod już jest w troszkę lepszym stanie. Pozostaje jeszcze powtórzyć tę czynność, aż od wypróż… opróżnienia mega-hiper-uber klasy. Po całej operacji uda Ci się rozbić jeden olbrzymi kloc na wiele malutkich klas. Czy można to tak zostawić? Absolutnie nie. Teraz masz otwartą drogę, aby posegregować te małe klasy, w grupy, w projekty. Może część z nich ma bardzo podobny kod i można je zastąpić jedną zgeneralizowaną wersją.
6. Klonowanie i ćwiartowanie
Sposób z powyższego punktu świetnie nadaje się do rozkładania klas na czynniki pierwsze. Ten natomiast świetnie się sprawdza, gdy napotkasz skomplikowaną metodę z olbrzymią ilością parametrów i warunków. Chwilę mi zajęło zanim wymyśliłem z której strony mogę ugryźć taki problem, aby cały czas mieć zapewniony stabilnie działający kod. Spójrz na kod:
public void Foo(ActionType actionType , string userName, object bar) { if(string.IsNullOrEmpty(userName)) { if(actionType == ActionType.Insert) { //add bar as anonymus } else if(actionType == ActionType.Delete) { //update bar as anonymus } } else if(userName == "admin") { //check statusType and do more things } // check next special users and normal users and do something }
Skoro są warunki to znaczy, że część logiki jest wykonywana w jednej ścieżce, a część w innej, w zależności od wartości parametrów. W takiej sytuacji. Zrób kopię takiej metody. Nazwij ją bardzo konkretnie, aby ograniczyć ilość parametrów i warunków, przykładowo BarAdderForAnonymusUser. Następnie usuwaj wszystko co nie jest używane podczas pojedynczego wywołania. Kroki powtarzaj, aż oryginalną klasę będziesz mógł usunąć, bo nigdzie nie będzie używana. Potem jeszcze zastosuj metodę z punktu 6, klasosranie i gotowe. Potwór pokonany.
7. Temp
Refaktoring zawsze składa się z wielu kroków. Jednym z nich będzie rozmieszczenie plików w odpowiednich projektach. Zdecydowanie polecam Ci odłożyć tę decyzję w czasie. Dlaczego? Abyś nie musiał zbyt wiele czasu poświęcać na przenoszenie kodu w tę i z powrotem. Lepiej w pierwszej kolejności mieć kod, podzielony na mniejsze kawałki.
W związku z tym. Stwórz nowy projekt o nazwie „Temp”, a następnie przenoś do niego, wszystkie, akceptowalnie małe, bądź wystarczająco czytelne, klasy. Dzięki temu prostemu zabiegowi, ilość kodu, wymagającego czyszczenia, cały czas będzie się zmniejszać w bazowym projekcie. Daje to świetną motywację, ponieważ będziesz widzieć, jak ilość pracy maleje.
Kiedy wszystkie pliki będą już małe i ładne to będzie Ci łatwiej wyznaczyć granice które pliki powinny powędrować do którego projektu. Metoda ta ma jeszcze jedną zaletę. Dzięki takiemu przepływowi kodu. Nie ma zapomnisz o żadnej klasie ponieważ na samym końcu musisz ten tymczasowy projekt opróżnić i wyrzucić.
8. Nazwy
Ten punkt w jawny sposób nie tyczy się tylko refaktoringu, lecz ogólnie programowania. Pisałem już na ten temat w krótkim poście o nazwach. W związku z tym to co jest napisane tu jest tylko uzupełnieniem tematu na płaszczyźnie refaktoringu.
Zmieniaj nazwę w taki sposób, aby po jej przeczytaniu było wiadomo co dana klasa/metoda robi. Jeżeli jakaś metoda waliduje, dodaje nowego użytkownika a następnie wysyła mu maila z linkiem potwierdzającym to pod żadnym pozorem nie powinna się nazywać AddUser. Odpowiednia nazwa dla takiej metody to ValidateAndAddUserAndSentConfirmationEmail. Zaraz, zaraz! Możesz mi powiedzieć, że przecież taka metoda jest zbyt długa, i nieczytelna. Zgadzam się! Jednakże, jest ona lepsza niż poprzednia, ale niech to będzie tylko pewien stan pośredni. Dzięki temu, widząc tę metodę, bez problemu dostrzeżesz, że coś jest nie tak. Możesz ją podzielić na 3 różne klasy: NewUserValidator, NewUserAdder, NewUserConfirmationEmailSender. A następnie wszystkich trzech użyć w klasie NewUserRegistration.
9. Usuwanie kodu
Praktycznie we wszystkich systemach. Bardzo głęboko, w półmroku, pod zwałami klas. Kryje się nieużywany, smutny, nikomu niepotrzebny kod. Kiedyś ktoś, go wywoływał i przekazywał mu parametry, instancjonował. Jednak z upływem czasu, zrobił się zgryźliwy i złośliwy. Wystrzeliwuje bełty uwagi w oczy programisty. Krzycząc. Czytaj mnie! Zrozum mnie! Refaktoruj! Uruchom!
Nie daj mu się zwieźć! Nie każdy kod wymaga refaktorowania. Zwłaszcza taki, który jest nieużywany. Taki kod należy bezzwłocznie usunąć, ponieważ każda usunięta linijka kodu, zmniejsza złożoność całego systemu. Dodatkowo zjada czas każdego programisty, który go czyta i próbuje go zrozumieć. Zatem jeżeli widzisz nieużywany kod to go usuń, jeśli okaże się, że była tam prawda objawiona to zawsze można go przywrócić z systemu kontroli wersji.
Na koniec
Ten wpis był dostępny, jako 9 różnych postów w cyklu „Jak refaktorować”. Jednak przyszło mi do głowy, że gdyby je połączyć to będzie z tego wartościowa całość. Mam wrażenie, że udało mi się to osiągnąć. Przepraszam za klikbajtowy tytuł, ale nie byłem w stanie się powstrzymać. To było silniejsze ode mnie.
Jeżeli uważasz ten wpis za wartościowy i zamierzasz użyć moje porady w praktyce to koniecznie mi o tym napisz. Tutaj pod spodem, albo na maila, po prostu jestem ciekawy. A może się z nimi nie zgadzasz, bo coś Ci się logicznie nie skleja? Też daj znać! Podyskutujemy.
7 Komentarzy
Piotr · 2018-02-02 o 10:30
Zgodze sie co do kilku punktow, jednak czesty commit to zazwyczaj samobojstwo przy pracy zespolowej. Repozytorium powinno byc bardzo czytelne , nie tylko dla 1 programisty, ktory jest autorem commita. Commit to zakonczenie etapu pewnej pracy – np. nad konkretnym zadaniem, gdzie mozna to zlinkowac np. z zadaniem z jira itp.
To o czym piszesz na pewno sprawdzic sie moze w repozytorium, nad ktorym pracuje tylko 1 programista :) a i tak sobie nie wyobrazam ogromu danych do sledzenia jakbym chcial czegos dalej szukac.
Jerzy Wickowski · 2018-02-02 o 11:32
@Piotr: Dzięki za komentarz. Odnośnie częstych komitów to oczywiście zależy od wielu czynników. Ja przykładowo bardzo lubię, gdy mogę prześledzić cykl rozumowania, zwłaszcza podczas refaktora, ale oczywiście pod kilkoma warunkami. Po pierwsze komity muszą być dobrze opisane. Po drugie, aby były robione na osobnym branczu i żeby następował jeden merge-commit, właśnie z id do zadania.
Natomiast, pozostaje jeszcze alternatywa, aby wypluwać dziesiątki lokalnie. Natomiast po skończonej pracy połączyć je w jeden zbiórczy komit. Mozna to zrobić używając komendy `git rebase –interactive`
Jakub Wojtyra · 2018-05-15 o 12:16
Przykład „4. Szybki return”, jest średni ponieważ autor zmienił wszystkie warunki, lepiej poprostu wyrzucić ify:
return code != null &&
code.Length == 7 &&
IsFormatCorrect(code);
Jerzy Wickowski · 2018-05-15 o 14:14
@Jakub: Hej. Przykład ma to do siebie, że powinen być prosty, nawet czasem trywialny. Chodzi o idee. Natomiast odnośnie czytelności to już kwestia gustu i przyzwyczajeń. Ja preferuję więcej prostych ifów z returnami, niż zagnieżdżone ify, czy jeden długi warunek.
Maciek S. · 2018-06-01 o 22:35
Właśnie chciałem o tym pisać, częste commity ok, ale na oddzielnym branchu. Osobiście jestem aktualnie na etapie „TODO” w trakcie zmian kodu, muszę wypróbować zaproponowane rzucanie własnego WTFException:) Co do nazewnictwa metod, z perspektywy młodszego stażem mam jeszcze czasem problem i jak się okazuje, czasem trzeba dobrze pomyśleć, żeby było czytelnie. Wiadomo, jak najmniejsze ciała metod, jedna odpowiedzialność, itd. Jeśli funkcja np. dodaje do bazy i wysyła maila – rozbiję na dwie mniejsze, a jak wtedy nazwać tą jedną zbiorczą, z której wywołam te dwie?:)
Jerzy Wickowski · 2018-06-02 o 05:26
Hej @Maciek. Dzięki za komentarz. Miło mi, że mogę pomóc.
Odnośnie nazewnictwa metod to zastanów się jaki jest jej cel biznesowy i spróbuj iść w tę stronę. Nazwa SaveToDatabaseAndSendEmail raczej mało mówi, ale FinishRegistration jest lepsza, a dodatkowo zyskujesz miejsce na wysyłanie SMS, czy innego komunikatu.
Dobra nazwa nie jest zła | Jerzy Wickowski · 2020-02-26 o 19:11
[…] Czasem można spotkać klasy-worki, niepozwalające określić swojej odpowiedzialności. Kod w nich robi wszystko, ale nic konkretnego. Rosną do niebotycznych rozmiarów. Ich pęcznienie powoduje dodatnie sprzężenie zwrotne. Im bardziej ich odpowiedzialność jest rozmyta, tym więcej rzeczy tam wpada, które z kolei rozmywają ją jeszcze bardziej. Weźmy na ruszt klasę CommonHelper, czy UserManager. Już po samej nazwie wiemy, że w środku jest takie „nie wiadomo co”. Jeżeli spotkasz coś takiego i zastanawiasz się jak to ogarnąć to zapraszam do posta o tym jak refaktorować. […]