Czy przydaje się mini-refaktoryzacja kodu w nadziei na poprawę jakości, czy jest to po prostu „przenoszenie kodu” bez większych korzyści?

12

Przykład

Natknąłem się na monolityczny kod, który robi „wszystko” w jednym miejscu - ładuje dane z bazy danych, pokazuje znaczniki HTML, działa jak router / kontroler / akcja. Zacząłem stosować SRP przenoszący kod bazy danych do własnego pliku, zapewniając lepsze nazewnictwo rzeczy i wszystko wyglądało dobrze, ale potem zacząłem mieć wątpliwości, dlaczego to robię.

Dlaczego warto refaktoryzować? Co jest celem? Czy to jest bezużyteczne? Jaka jest korzyść? Zauważ, że w większości zostawiłem plik monolityczny bez zmian, ale zmieniłem tylko mniejszą część, która była odpowiednia dla obszaru, w którym musiałem trochę popracować.

Oryginalny kod:

Aby dać konkretny przykład, natknąłem się na ten fragment kodu - ładuje specyfikacje produktu albo przez znany identyfikator produktu, albo przez identyfikator wersji wybranej przez użytkownika:

if ($verid)
    $sql1 = "SELECT * FROM product_spec WHERE id = " . clean_input($verid);
else
    $sql1 = "SELECT * FROM product_spec WHERE product_id = " . clean_input($productid) ;
$result1 = query($sql1);
$row1 = fetch_array($result1);
/* html markup follows */

Refaktoryzacja:

Ponieważ wykonuję pewne prace wymagające zmiany rzeczy w tej konkretnej części kodu, zmieniłem go, aby używał wzorca repozytorium, i zaktualizowałem go, aby używał obiektowych obiektów MySQL:

//some implementation details omitted 
$this->repository = new SpecRepository($mysql);
if ($verid)
    $row1 = $this->repository->getSpecByVersion($verid);
else 
    $row1 = $this->repository->getSpecByProductId($productid);
/* html markup follows to be refactored or left alone till another time*/

//added new class:
class SpecRepository extends MySqlRepository
{

    function getSpecByVersion(int $verid)
    {
        return $this->getMySql()->paramQuery("
            SELECT * FROM product_spec WHERE id = ?
        ", $verid)->getSingleArray();
    }

    function getSpecByProductId(int $productid)
    {
        return $this->getMySql()->paramQuery("
            SELECT * FROM product_spec WHERE product_id = ?
        ", $productid)->getSingleArray();
    }
}

Powinienem to zrobić?

Patrząc wstecz na zmiany, kod wciąż tam jest, kod z tą samą funkcjonalnością, ale w różnych plikach, różnych nazwach, miejscach, przy użyciu bardziej obiektowego stylu niż procedury. W rzeczywistości śmiesznie jest zauważyć, że zrekonstruowany kod wygląda na znacznie bardziej rozdęty, mimo że ma tę samą funkcjonalność.

Przewiduję kilka odpowiedzi mówiących: „jeśli nie znasz powodów, dla których dokonujesz refaktoryzacji, nie rób tego”, a być może zgodzę się. Moim powodem jest poprawa jakości kodu w czasie (i mam nadzieję, że zrobię to, przestrzegając SRP i innych zasad).

Czy to wystarczające powody, czy marnuję czas na „przestawianie kodu” w ten sposób? Ogólnie mówiąc, refaktoryzacja przypomina trochę kroczenie po wodzie - wymaga czasu i staje się bardziej „oddzielona”, jeśli chodzi o SRP, ale pomimo moich dobrych intencji nie czuję się, jakbym robił niesamowite ulepszenia. Dlatego zastanawiamy się, czy najlepiej zostawić kod jak poprzednio, a nie refaktoryzować.

Dlaczego przede wszystkim dokonałem refaktoryzacji?

W moim przypadku dodałem nową funkcjonalność dla nowej linii produktów, więc muszę albo przestrzegać istniejącej struktury kodu dla podobnych linii produktów, albo napisać własną.

Dennis
źródło
OT, ale należy wziąć pod uwagę, że Select * from ..można to uznać za anty-wzór. Zobacz stackoverflow.com/q/3639861/31326
Peter M
1
@PeterM: Jeśli nie piszesz ORM, w takim przypadku select *staje się „najlepszą praktyką”.
Robert Harvey,
@RobertHarvey W tym przypadku mam wątpliwość, dlaczego ty piszesz własną ORM: D
Peter M
Zrobiłem refaktorów dla bardziej nieistotnych przyczyn. Zmniejszenie długu technicznego jest dobre, gdy tylko korzyści przewyższą koszty. Wygląda na to, że to twoja sprawa. Pytanie brzmi: czy masz odpowiednie testy, aby upewnić się, że kod nadal działa zgodnie z oczekiwaniami?
Laiv
1
Wiarygodność powinna być dominującą miarą przy refaktoryzacji IMO, a nie łatwość konserwacji, która odnosi się do zmienności, ponieważ niezawodność zmniejsza przede wszystkim przyczyny zmian. Poprawia stabilność, a kod, który jest całkowicie stabilny i niezawodny i spełnia wszystkie jego potrzeby, wiąże się z niewielkimi kosztami utrzymania, ponieważ ma bardzo niewiele powodów, aby kiedykolwiek zmieniać. Staraj się zmniejszyć przyczyny zmian, zamiast starać się, aby zmiana była jak najłatwiejsza. Poszukiwanie tego pierwszego będzie również dawało część tego drugiego.

Odpowiedzi:

13

W konkretnym podanym przykładzie refaktoryzacja mogła nie być konieczna, a przynajmniej jeszcze nie. Widziałeś jednak, że w przyszłości może istnieć bałaganiarski kod, który doprowadziłby do dłuższego i bardziej żmudnego refaktoryzacji, więc przejąłeś inicjatywę i posprzątałeś wszystko.

Powiedziałbym również, że korzyścią, którą tutaj zdobyłeś, był kod, który jest łatwiejszy do zrozumienia, przynajmniej z mojego punktu widzenia, jako ktoś, kto nie zna twojej bazy kodu.


Ogólnie:

Czy refaktoryzacja ułatwia innym programistom zrozumienie twojego kodu?

Czy refaktoryzacja ułatwia ulepszenie kodu?

Czy refaktoryzacja ułatwia utrzymanie kodu?

Czy refaktoryzacja ułatwia debugowanie?

Jeśli odpowiedź na którekolwiek z nich brzmiała „tak”, to było warto i było czymś więcej niż tylko „przenoszeniem kodu”?

Jeśli odpowiedź na wszystkie te pytania brzmiała „nie”, to być może nie trzeba jej było refaktoryzować.

Ostateczny rozmiar podstawy kodu może być większy pod względem LoC (chociaż niektóre refaktoryzacje mogą zmniejszyć bazę kodu przez usprawnienie redundantnego kodu), ale nie powinno to być czynnikiem, jeśli istnieją inne korzyści.

FrustratedWithFormsDesigner
źródło
9

Refaktoryzacja powoduje wzdęcia, jeśli demontujesz monolit.

Jednak już znalazłeś korzyść.

„W moim przypadku dodałem nową funkcjonalność dla nowej linii produktów”.

Nie można bardzo łatwo dodać funkcjonalności do monolitu bez zepsucia.

Powiedziałeś, że ten sam kod pobiera dane i robi znaczniki. Świetnie, dopóki nie chcesz zmienić, które kolumny wybierasz i którymi manipulujesz, aby wyświetlać się w określonych warunkach. Nagle twój kod znaczników przestaje działać w określonym przypadku i muszą Refactor.

Diakon
źródło
tak, ale mogę dodawać do monolitu nawet dla nowej funkcjonalności :) tj. dodawałbym nowy if/elseblok i postępowałem zgodnie z oryginalnym stylem kodu, aby dodawać instrukcje SQL, a następnie umieszczałem nowy znacznik HTML w tym samym pliku monolitu. Aby zmienić kolumny, znalazłbym blok if / else odpowiedzialny za umieszczenie wierszy SQL i edytuję ten SQL, aby wprowadzić zmiany w kolumnach bez rozbijania tego pliku.
Dennis,
Po ponownym podejściu prawdopodobnie najpierw przejdę do bloku if / else w monolicie, aby zobaczyć, że muszę przejść do osobnego pliku repozytorium, aby zmienić SQL. Lub gdybym ostatnio pracował nad tym monolitem, wiedziałbym, że zamiast tego przejdę bezpośrednio do pliku repozytorium, omijając monolit. Lub gdybym przeszukał moją bazę kodu w poszukiwaniu określonego SQL, wyszukiwanie umieściłoby mnie w nowym pliku repozytorium również z pominięciem monolitu
Dennis
2

Rozważam refaktoryzację, jeśli wiem, że będę musiał pracować miesiącami, a nawet latami nad tym projektem. Jeśli muszę coś zmienić tu i tam, to opieram się chęci zmiany kodu.

Preferowanym sposobem przeprowadzania refaktoryzacji jest posiadanie kodu podstawowego z pewnego rodzaju automatycznymi testami. Mimo to muszę bardzo uważać na to, co zmieniam, aby upewnić się, że wszystko działa jak wcześniej. Wkrótce stracisz zaufanie, jeśli dostarczysz błędy w obszarach innych niż te, które miałeś pracować.

Cenię refaktoryzację z następujących powodów:

  • Rozumiem kod lepiej
  • Usuwam zduplikowany kod, więc jeśli mam błąd w jednym miejscu, nie muszę polować wszędzie tam, gdzie kod został wklejony
  • Tworzę klasy, które mają dobrze zdefiniowaną rolę. Na przykład mogę ponownie użyć tego samego repozytorium do uwodnienia strony HTML, kanału XML lub zadania w tle. Nie byłoby to możliwe, gdyby kod dostępu do bazy danych istniał tylko w klasie HTML
  • Zmieniam nazwy zmiennych, metod, klas, modułów, folderów, jeśli nie pasują one do obecnej rzeczywistości; I commentkod poprzez nazwy zmiennych, nazwy metod
  • Rozwiązuję złożone błędy za pomocą kombinacji testów jednostkowych i refaktoryzacji
  • Mam elastyczność w zastępowaniu całych modułów, pakietów. Jeśli bieżąca ORM jest przestarzała, wadliwa lub nieobsługiwana, łatwiej jest ją wymienić, jeśli nie jest rozłożona na cały projekt.
  • Odkrywam nowe możliwości lub ulepszenia, które powodują propozycje dla klienta.
  • Stworzyłem komponenty wielokrotnego użytku. Była to jedna z największych korzyści, ponieważ praca wykonana w tym projekcie mogła zostać ponownie wykorzystana dla innego klienta / projektu.
  • Często zaczynam od najgłupszego, zakodowanego, zwariowanego rozwiązania, a później, gdy się więcej nauczę, zmieniam je później. Ten późniejszy moment może być dzisiaj, a może po kilku dniach. Nie chcę spędzać zbyt dużo czasu architectna rozwiązanie. Stanie się tak podczas pisania - przepisywanie kodu tam iz powrotem.

W idealnym świecie refaktoryzacje powinny być weryfikowane za pomocą testów jednostkowych, testów integracyjnych itd. Jeśli nie ma, spróbuj je dodać. Również niektóre IDE mogą bardzo pomóc. Zwykle używam wtyczki, która kosztuje. Chcę spędzać mało czasu na refaktoryzacji i być bardzo wydajnym w tym zakresie.

Wprowadziłem również błędy. Rozumiem, dlaczego niektórzy QA pytają o are you guys doing refactoring? because everything stopped working!to ryzyko, które musi objąć zespół i zawsze musimy być wobec tego transparentni.

Przez lata odkryłem, że ciągłe refaktoryzowanie poprawiło moją wydajność. O wiele łatwiej było dodać nowe funkcje. Również duże przebudowy nie wymagały całego przepisywania, co często zdarzało się, gdy baza kodu nie była dostosowana do ewolucji produktu. To także zabawa.

Adrian Iftode
źródło
Jest to bardzo bliskie głosowania w dół - w „Programistach komputerowych” nie ma „ja”. Przez „ja” czy ty „ja” (głosowanie w dół) czy masz na myśli programistów pracujących nad kodem teraz iw przyszłości?
mattnz,
Zauważyłem to też ja, ale zapisałem to. Przez wiele lat pracowałem jako solowy programista i kiedy napisałem tę odpowiedź, powtórzyłem głównie z tego doświadczenia. Mógłbym wymienić Iz tochociaż.
Adrian Iftode,
1

Myślę, że pytanie, które musisz sobie zadać, to nie tyle: „Czy jest jakaś korzyść?” ale „Kto zapłaci za tę pracę?”

Wszyscy możemy spierać się o postrzegane korzyści do końca dni, ale jeśli nie masz klienta, który wierzy w te korzyści i ceni je na tyle, aby zapłacić za zmiany, nie powinieneś ich robić.

To wszystko jest zbyt łatwe, gdy jesteś gdzieś w zespole deweloperów, aby zobaczyć kod i chcesz go zmodyfikować, aby był „lepszy”. Ale przypisywanie wartości „lepszemu kodowi”, gdy produkt już działa, jest naprawdę trudne.

Rzeczy takie jak „szybszy rozwój nowych funkcji” nie zmniejszają tego, ponieważ są po prostu zmniejszonymi kosztami. Musisz wygenerować sprzedaż.

Ewan
źródło
1

Moim zdaniem refaktoryzacja to dobra inwestycja.

W przeciwnym razie wkrótce otrzymasz zadłużenie techniczne (nierozwiązane problemy, zły kod, który działa tylko pod pewnymi warunkami itp.). Techniczne długi wkrótce będą coraz większe, dopóki oprogramowanie nie będzie dłużej możliwe do utrzymania.

Ale aby refaktoryzacja działała, musisz również zainwestować w testy. Powinieneś tworzyć testy automatyczne, najlepiej powinieneś mieć testy jednostkowe i testy integracyjne. Uniemożliwia to złamanie istniejącego kodu.

Jeśli musisz przekonać swojego szefa lub współpracowników, powinieneś przeczytać kilka książek o zwinnych metodach (np. SCRUM) i rozwoju opartym na testach.

bernie
źródło