Refaktoryzacja

refactoring

Refaktoryzacja kodu to temat rzeka. Skupię się dzisiaj na poprawie czytelności kodu. Skąd pomysł? Ostatnio w pracy trzeba było dodać do programu nową funkcjonalność. Nic prostszego czyż nie? No niekoniecznie. Gdy zobaczyłem kod (ponad 2 tys. linii kodu) przeraziłem się. Do tego mnóstwo powielania kodu i wszechobecny chaos. Pomieszanie z poplątaniem, ogólnie tragedia. I co z tym fantem zrobić. Miałem 2 opcje do wyboru. Albo zostawić to tak jak jest i dodawać po prostu metody do nowej funkcjonalności w tym samym przeogromnym pliku, albo zrefaktoryzować ten kod od razu. Wybrałem tą drugą opcje bo co się odkłada na zaś, to czasem to “zaś” zamienia się w nigdy.

Refaktoryzacja – od czego zacząć?

Po pierwsze analiza kodu pod względem powielania tych samych czynności. W celu zobrazowaania problem przedstawię fragment przykładowego kodu.

I co sądzicie? Brzydko prawda? A to tylko fragment kodu w tej klasie zwanej DataAdapter znajdują się wszystkie zapytania i wyniki przez nie zwracane (wszystkie listy, operacje kasowania, dodawania itd). Aż ciężko się patrzy a co dopiero rozbudowywać taką aplikacje. Masakra. Ale właśnie tu z pomocą przychodzi nam refaktoryzacja.

Przede wszystkim musimy wydzielić odpowiedzialności. Zbity kod w jedną całość nie jest ani wygodny w edycji ani przejrzysty. Może i mamy wszystko pod ręką, mamy jeden plik, ale czy aby na pewno o to chodzi? No nie sądzę. Jest to kod z początków mojej przygody z C# (to tak na usprawiedliwienie ;p).

Struktury plików nawet nie ma co przedstawiać bo był tu zaledwie 1 plik DataAdapter.cs Database.cs, w którym definiujemy połączenie z bazą danych.

Do dzieła!

Pierwsze co rzuca się w oczy to zapis zapytań:

Nie wygląda to za ładnie, tym bardziej, że posiadamy teraz możliwość takiego zapisu:

Od razu lepiej. Zaczynałem pisanie kodu w VS Express 2013 i w tej wersji niestety taki zapis nie będzie działał.

Zacznijmy od stworzenia klasy (lub klas), w której będziemy trzymać wszystkie zapytania odnoszące się do danego modułu lub obiektu naszej aplikacji. Dla przykładu:

  • Objects.cs – zapytania na obiektach, np. selectObject(int object_id)
  • Users.cs – zapytania na użytkownikach, np. deleteUser(int user_id)
  • Logs.cs – zapytania na logach, np. fetchLogs(DateTime from, DateTime to)
  • etc.

Super, posiadamy już klasy dla naszych zapytań. Dla większej przejrzystości utwórzmy folder Queries, w którym to będziemy przetrzymywać wszystkie klasy zapytań. Przenosimy zapytania do odpowiednich klas. I tak w naszym przykładzie będzie to plik Objects.cs i wygląda on następująco:

Wygląda to całkiem ładnie. Spytacie dlaczego static, dlatego żeby nie musieć tworzyć za każdym razem przy chęci użycia zapytania nowego obiektu. Wywołanie zapytania jest niezwykle proste wystarczy w części kodu gdzie chcemy użyć zapytania wywołać:

Wyszedłem trochę na przód z _db.runQuery ale do tego za chwilę dojdziemy.

Separacja części wspólnych

To bardzo ważny etap, który de facto powinien być wykonywany na bieżąco w trakcie pisania kodu. Ale jeśli tego nie zrobiliśmy to należy to zrobić podczas refaktoryzacji. Spójrzmy na nasz kod, co powtarza się często …. chwila na zastanowienie …. i do dzieła. Zapytania, które zwracają jakiś wynik lub go nie zwracają będą pojawiać się dość często, stwórzmy zatem interfejs nazwijmy go IDatabase. Zawrzemy w nim szkielety naszych metod:

Mamy w nim 2 metody, jedna – runQueryNoDataResult – zwraca pusty wynik (np. przy operacji kasowania) druga – runQuery – natomiast zwraca nam jakieś dane (np. listę użytkowników). Następnie tworzymy obiekt implementujący nasz interfejs obsługi bazy danych.

W klasie Database inicjujemy połączenie z bazą danych – ale nie będę tu wszystkiego wklejał, bo równie dobrze mógłbym cały kod wkleić i go objaśniać a nie o to chodzi. Następnie definiujemy ciała naszych szkieletów metod zdefiniowanych w interfejsie. Wkleję jedną z metod (druga analogiczna).

No i super. Posiadamy teraz w jednym miejscu wszytko co do tej pory mieliśmy kilkakrotnie powielane. I taki efekt nam chodziło.

Końcówka refaktoryzacji

Zbieramy wszytko do ?. Odseparowaliśmy w zasadzie wszystko co mogliśmy. Posiadamy już bardziej obszerną strukturę plików i mimo, że jest ich więcej cały projekt stał się bardziej czytelny. Jak zatem wygląda aktualnie nasz plik DataAdapter, od którego zaczynaliśmy?

Zdecydowanie lepiej, czyż nie? A jak zatem wygląda struktura plików. Dla przypomnienia na początku posiadaliśmy tylko 2 pliki. Teraz jest ich trochę więcej doszły do tego foldery, ale zdecydowanie łatwiej teraz rozbudowywać nasza aplikację.

refaktoryzacja

Podsumowanie

Refaktoryzacja z zachowaniem dobrych wzorców jest niezwykle efektywnym procesem. Oczywiście łatwiej byłoby pisać od razu przejrzysty kod bez konieczności refaktoryzacji, ale powiedzmy sobie szczerze czy istnieje taka aplikacja? Nie wydaje mi się. Zawsze jest coś co można poprawić. Warto poświęcić trochę czasu na poprawę jakości i przejrzystości naszego kodu, zwłaszcza jeśli planujemy go dalej rozwijać w przyszłości.

  • Bleakerin

    Cześć,
    Dość przystępny przykład :). Jakie jest uzasadnienie użycia w tym przykładzie interfejsu?

    • http://www.gemustudio.com blaze

      Część, dzięki 🙂 Co do intefrejsu to tak jak teraz patrzę to faktycznie może tu jest niepotrzebny. Wystarczyłaby klasa abstrakcyjna tak myślę:)

  • Zaba

    Cześć. Nigdy nie zostawiaj gwiazdki w zapytaniach sql.
    SELECT * FROM objects WHERE id {0}
    Zawsze lepiej mieć dokładnie sprecyzowane kolumny w zapytaniu.

    • http://writestoft.wordpress.pl Piotr Perak

      Chyba, że chcesz pobrać wszystkie kolumny?

    • http://www.gemustudio.com blaze

      Część. Hmm …. coś w tym jest. Wychodzi przyzwyczajenie z zapytań gdzie były do pobrania wszystkie kolumny a to zdecydowanie upraszczało zapytanie :p

  • 0xmarcin

    Ja bym tutaj dał taką radę że kod operujący na DB powinien jasno i wyraźnie mówić o tym co robi, np zamiast ukrywać zapytanie w innej metodzie być może lepiej przepisać kod na coś w stylu:

    Dictionary result = adonetFacade.SelectToDictionary(@”
    select *
    from froo_table
    where blah…
    “);

    Lub jeżeli chcesz mieć jawne mapowanie z result set:

    Dictionary result = adonetFacade.Select(@”
    select *
    from froo_table
    where blah…
    “, (ResultSet dr) => {
    object_params.Add(“object_id”, Convert.ToInt64(dr[“id”]));
    object_params.Add(“object_name”, Convert.ToString(dr[“name”]));
    object_params.Add(“param”, Convert.ToInt64(dr[“value”]));
    object_params.Add(“param”, Convert.ToInt64(dr[“value”]));
    });

    Lub jeszcze lepiej niech od razu mapuje do DTO (czyli naszej klasy z danymi). Idąc w tym kierunku można szybko dojść do wniosku że piszemy własnego Dozera – więc czemu by nie skorzystać z już napisanego…

    I tym sposobem kod naprawdę się upraszcza, bo sporej jego części nie musimy wcale pisać.

    • http://www.gemustudio.com blaze

      Dzięki za feedback. Moim zamiarem było wyjaśnienie na czym polega refaktoryzacja na w miarę prostym przykładzie. Akurat w pracy refkatoryzuję obsługę bazy danych stąd przykład z bazą.

  • http://writestoft.wordpress.pl Piotr Perak

    Cześć. Jeszcze dużo sporo można poprawić.

    Na szybko to metoda runQuery powinna wygladać tak:
    https://gist.github.com/PiotrPerak/bf68ee725a714ea4f1ba2678d158c975
    Jak widzisz można zredukować jedno wcięcie dzięki trzymaniu dwóch usingów jeden za drugim. Connection.Open() robimy przed Execute – zawsze jak najpóźniej. Natomiast Close() nie musimy wołać. Zrobi to Dispose().

    Oprócz tego masz bład w linii 5 w akapicie Końcówka refaktoryzaji. Co jak RunQuery zwróci null? Swoja droga nie powinno. Powinno zwracać readera i wtedy ta metoda będzie miała 4 linie.

    Obsługa wyjatkow powinna być zrobiona wyżej. Tak ukrywasz błędy i wymuszasz na kliencie sprawdzanie, czy rezultat nie jest nullem. A dodatkowo nie logujesz stacktrace’a.

    No i jeszcze się przyczepię do nie trzymania się konwencji przyjętej w C# – nazwy metod z wielkich liter.

    Nie znam Postgres-a, ale czy nie brakuje znaku = za WHERE id?

    • http://www.gemustudio.com blaze

      Cześć. Dzięki za cenne uwagi. Faktycznie Twój kod wygląa zdecydowanie lepiej 🙂 Tak zapomniałem o znaku równości :p jest to przykład pisany z ręki nie sprawdzony (mój błąd), co nie jest oczywiście żadnym usprawieliwieniem. Takie błędy nie powinny się pojawiać. Już poprawiłem. Na przyszłość wszystko dokładniej sprawdzę.

  • eN

    Kod przed refaktoryzacją wcale tak źle nie wyglądał, a przynajmniej to był znacznie mniejszy problem niż:
    – używanie dynamicznych zapytań zamiast parametrycznych
    – masę boxingów przez rzutowanie intów na object w słowniku
    za takie rzeczy wysłane na produkcje to się wiesza do góry nogami