Refactoring - Jak robić i kiedy merge?

Temat trochę pokrewny do poprzedniego o długu technicznym, ale jednak tym razem chodzi mi o coś szerszego.

Powiedzmy, że zespół osiągnął konsensus, że kod to kupa mułu i podjął decyzję: REFAKTORUJEMY. Np. zamieniamy wszędobylski static i singletony na DI.

No i teraz jak to zrobić dobrze?

1. Bierzemy tego kto zaproponował pomysł refaktoringu, dajemy mu pół roku i biedaczysko poprawia aż będzie dobrze, oczywiście w swojej gałęzi. Jak skończy - robimy wielkie review, dajemy kolejne 3 miesiące na uwzględnienie uwag i naprawienie konfliktów i po kolejnych 3 miesiącach w końcu mamy merge i możemy świętować.

2. Przerabiamy kod stopniowo - najpierw dodajemy nowa infrastrukturę, potem przerabiamy komponent po komponencie, każdy zespół przerabia swój komponent na nową metodę. Jak już wszystkie komponenty są ok, wywalamy ewentualnie resztki starego kodu, który nie jest już potrzebny i robimy merge.

3. Robimy jeden komponent, merge, oczekujemy trochę na stabilizację, robimy kolejny, znowu merge itd. W międzyczasie w kodzie musimy utrzymywać infrastrukturę dla obu sposobów - czyli niektóre rzeczy mają statici a niektóre są przeniesione na nowy framework. Możliwe, że po drodze trzeba stworzyć jakieś warstwy kompatybilności. Sprzątamy dopiero na końcu jak wszystkie komponenty są przeniesione.

4. Inne. Jakie?

Najpierw napiszcie testy! (chyba ze juz macie na wszystko) Najlepiej e2e zeby móc sobie orać kod dowolnie. Potem starcie sie kolejno wydzielać komponenty. Im częściej merge tym lepiej, bo dzięki temu nikt nie będzie "z tyłu"

Nie widziałem, żeby 1 zadziałało (co nie znaczy, że się nie da). 2 daje efekty bardzo późno, więc jak nie ma wsparcia z góry, to pewnie zostanie ubite w pewnym momencie. 3 jest okej, ale łatwo jest w połowie pracy stwierdzić, że już wystarczy i zostają dwa niekompatybilne rozwiązania.

Bez kodu trudno wyczuć, ale zacząłbym od podejścia 3.

1 i 2 nigdy się nie uda na większych projektach. Tylko 3 ale z dodatkiem testów. Więcej można przeczytać w książce "Praca z zastanym kodem. Najlepsze techniki".

Zespoł musi być świadomy, że to jest potrzebne (jak 1 osoba na 10 w zespole widzi problem to dużo nie zdziała w pojedynkę). W refaktoryzacji nie ma pojęcia "koniec", to proces który powinien być codzienną pracą programisty. Testy to bardzo dobre narzędzie do refaktoru, nawet najlepszy kod bez testów to legacy/zastany kod. Jeżeli napiszecie nowy kod który wydaje Wam się lepszy, fajniejszy itp. ale bez testów - to za kilka lat znowu będziecie musieli przechodzić ten sam proces. Testy jednostkowe i ew. integracyjne tylko dla nowego kodu (ale bez rygoru minimalnego pokrycia, to często prowadzi do testów które nic nie testują), dla legacy często nie da się takich napisać więc wystarczą testy end-to-end. Dodatkowo wszystkie narzędzia które pomagają w trzymaniu jakości kodu np. Sonar i mierzeniu tego co zostało poprawione (zespoł jak widzi wykres, że jest mniej duplikacji, mniej staticów itp. to jest bardziej zmotywowany).

Również głosuję na 3, żyjemy w czasach agilowych, które nastały właśnie dlatego że big bangowe podejście 1) prawie nigdy się nie sprawdzało bez względu czy tyczy się to refaktoringu czy nowego developmentu. Chcemy jak najszybciej uzyskać feedback że nic nie popsuliśmy i że zmierzamy w dobrym kierunku. Kroimy malutki kawałeczek, przepisujemy go tak by testy były dalej zielone, jak nie mamy testów to pierw testy piszemy, wrzucamy na produkcję i zbieramy metryki(validated learning), robimy retrospekcję, jak wszystko śmiga pięknie powtarzamy całą pętle.

Markuz napisał(a):

1 i 2 nigdy się nie uda na większych projektach. Tylko 3 ale z dodatkiem testów. Więcej można przeczytać w książce "Praca z zastanym kodem. Najlepsze techniki".

E tam się nie uda...
Ja właśnie sam robię refaktoring typu 1, tylko że nie miałem pół roku tylko jakieś 2 tygodnie (miałem dziś skończyć, oczywiście nie skończyłem :D ). Testów nie ma, chyba że je napiszę, ale nie planuję, bo nigdy nie skończę. Review to też może mi zrobi 1-2 osoby, bo nikt więcej nie rozumie co się tam dzieje. Projekt na kilkadziesiąt klas, dość złożony, dużo wielowątkowości, algorytmiki, konwolucyjnych sieci neuronowych, mieszany kod C++/CUDA, przetwarzanie obrazów. W międzyczasie naprawiam biblioteki zależne i budowanie paczek debianowych :)

Pozostaję optymistą :)

Krolik napisał(a):

1. Bierzemy tego kto zaproponował pomysł refaktoringu, dajemy mu pół roku i biedaczysko poprawia aż będzie dobrze, oczywiście w swojej gałęzi. Jak skończy - robimy wielkie review, dajemy kolejne 3 miesiące na uwzględnienie uwag i naprawienie konfliktów i po kolejnych 3 miesiącach w końcu mamy merge i możemy świętować.

Bardzo optymistyczne założenie o szampanie po roku. Ale bardziej niesamowite byłoby to review półrocznej pracy człowieka posadzonego sam na sam na pół roku z kodem. Oczywiście testów jak nie było wcześniej tak po pół roku też nie będzie.

• Robimy porządne pokrycie testami akceptacyjnymi, integracyjnymi, e2e
• Wystawiamy jakiś ogólny interfejs modułu, tak żeby pomiędzy sobą moduły w ogóle nie wiedziały o internalsach i komunikowały się tylko i wyłącznie tym abstrakcyjnym interfejsem
• Przerabiamy ten moduł i podmieniamy

Czyli generalnie opcja 3. Plus jest taki że szybko werfykujesz czy to w ogóle działa. I nie ma też takiego problemu, że dochodzą nowe ficzery i trzeba je klepać 2 razy (raz w nowym raz w starym systemi). Im szybciej wypchniesz coś co działa, tym lepiej.

Ale dużo zależy od tego jak wygląda aktualny stan i co chcecie osiągnąć. Bo może nie warto trzymać się starego interfejsu.?

Opcja 1 czy 2 mają sens jeśli mozecie sobie pozwolić na freeze produktu (bo np. jest tam już tylko maintenance), szczególnie kiedy wiecie ze chcecie to na pewno zaorać i zrobić od nowa. Jeśli da sie coś "uratować" to opcja 3 zawsze będzie lepsza.

Robiłem ostatnio taki "refaktor" gdzie nie dało się uratować nic. Projekt klepany z 10 lat temu, Spring 2, Hibernate w xmlach, a do tego jeszcze jakiś webowy framework który umarł z 5 lat temu i cała logika aplikacji sklejona z tym webowym frameworkiem albo pisane "na piechote". Jakieś oczywistości jak zwrócenie z kontrolera InputStreama pisane ręcznie przez pisanie w pętli do httpservletresponse itd.
Oczywiście monolit więc nawet nie bardzo jak przepisywać coś po kawałku, bo nie da się wydzielić modułu.
W takiej sytuacji po prostu nie da się inaczej niż 1/2.

Bierzemy tego kto zaproponował pomysł refaktoringu, dajemy mu pół roku i biedaczysko poprawia aż będzie dobrze, oczywiście w swojej gałęzi

Przez pół roku to można przepisać całą aplikację, a nie tylko zrefaktorować...

Dajcie mu pół tygodnia, niech zrefaktoruje moduł, zmerdżuje się, potem następny moduł itp.

Jak skończy - robimy wielkie review, dajemy kolejne 3 miesiące na uwzględnienie uwag i naprawienie konfliktów i po kolejnych 3 miesiącach w końcu mamy merge i możemy świętować.

= Waterfall.

Nie sądzę, żeby to się sprawdziło.

Przerabiamy kod stopniowo - najpierw dodajemy nowa infrastrukturę, potem przerabiamy komponent po komponencie, każdy zespół przerabia swój komponent na nową metodę. Jak już wszystkie komponenty są ok, wywalamy ewentualnie resztki starego kodu, który nie jest już potrzebny i robimy merge.

No, tu już rozsądniej. Chociaż merge ja bym robił jak najszybciej (Robimy jeden komponent, merge, oczekujemy trochę na stabilizację, robimy kolejny, znowu merge), żeby od razu wykryć, czy coś działa, czy nie.

oczekujemy trochę na stabilizację

Napiszcie sobie testy, to wtedy od razu będziecie wiedzieli, czy nie ma jakichś bugów itp.

Przy refacoringu takich starych kobył mi się sprawdza algorytm postępowania w stylu:

1. Staram się napisać testy pokrywające wszystkie "odnogi", przypadki refaktorowanego kodu.
2. Dopiero wtedy siadam do poprawiania.
3. Na bieżąco sprawdzam/aktualizuje testy.
4. Przypisuję wyjadaczy projektowych do code review swojego PR (bo czasami wywalasz coś myśląc, że "po co komu to tutaj potrzebne", a gość pracujący 5 lat w projekcie mówi Ci, że jakiś syf, o którym nigdy nie słyszałeś z tego korzysta i bez tego wszystko je**ie).

Dosyć żmudny sposób, ale już się przejechałem na tym, że jak coś się poprawia w projekcie ze sporym długim technicznym, to jest to najrozsądniejszy i najbardziej opłacalny na dłuższą metę sposób.
Chociaż cięższe od samego sposobu jest raczej przekonanie biznesu, że taka robota jest warta zachodu mimo, że nie widzą na oczy żadnych efektów i nie dowozimy im nowych, bajeranckich ficzerów. :D

Do mniejszego refactoringu najlepiej wprowadzić w życie Boy Scout Rule (lub zależnie od poglądów ideologicznych po prostu Scout Rule). Innymi słowy- poprawiamy kod w ramach codziennej pracy nad taskami.

Jeśli chodzi o większe zmiany to nie obejdzie się bez wkładu/akceptacji biznesu, a szczegóły naprawdę zależą od natury zmian, projektu i kultury pracy. Moim zdaniem najzdrowszym podejściem jest zaangażowanie większości programistów w projekcie. Jeśli w projekcie używa się np. Scrum to poświęcamy całe iteration na refactoring- żadnych nowych funkcjonalności, wszystkie taski napisane pod poprawę istniejącego kodu/architektury.

Obecnie u mnie w pracy będzie to miało miejsce więc zapowiada się ciekawie :)

Na temat refactoringu, architektury, testowania przede wszystkim trzeba mieć pojęcie. Jeśli piszesz, że chcecie wymienić wszędobylski static i singletoy na DI to nie brzmi to dobrze. Jeśli sami stworzyliście tego potwora i teraz sami, chcecie to zrefactoryzować, to raczej to nie wypali. Widziałem już jak w jednej firmie, senior zrobił refactoring, z lazanii na inny rodzaj spaghetti. Dobry kod przede wszystkim dobrze się testuje, a na refactoring są też są przydatne pateny np open host, anti corruption layer.