Pogodzenie reguły skautowej i oportunistycznego refaktoryzacji z recenzjami kodu

Jestem wielkim zwolennikiem zasady skautów :

Zawsze sprawdzaj moduł w czystszym stanie niż wtedy, gdy go sprawdziłeś. ”Bez względu na to, kto był oryginalnym autorem, co, jeśli zawsze podejmiemy jakiś wysiłek, nie ważne jak mały, aby ulepszyć moduł. Jaki byłby wynik? Myślę, że gdybyśmy wszyscy postępowali zgodnie z tą prostą zasadą, widzieliśmy koniec nieustannego niszczenia naszych systemów oprogramowania. Zamiast tego, nasze systemy stopniowo stawały się coraz lepsze wraz z ich ewolucją. Widzieliśmy także zespoły dbające o system jako całość, raczej niż tylko osoby dbające o swoją małą, małą część.

Wierzę również w pokrewną ideę oportunistycznego refaktoryzacji :

Chociaż istnieją miejsca na zaplanowane działania refaktoryzacyjne, wolę zachęcać do refaktoryzacji jako działania oportunistycznego, wykonywanego zawsze i wszędzie, gdzie trzeba wyczyścić kod - przez kogokolwiek. Oznacza to, że w dowolnym momencie ktoś widzi kod, który nie jest tak przejrzysty, jak powinien, powinien skorzystać z okazji, aby go naprawić natychmiast - a przynajmniej w ciągu kilku minut

W szczególności zwróć uwagę na następujący fragment artykułu z refaktoryzacji:

Obawiam się wszelkich praktyk programistycznych, które powodują tarcie w oportunistycznym refaktoryzacji ... Mam wrażenie, że większość zespołów nie robi wystarczająco refaktoryzacji, dlatego ważne jest, aby zwracać uwagę na wszystko, co zniechęca ludzi do robienia tego. Aby pomóc Ci to wypłukać, miej świadomość, że kiedykolwiek zniechęca Cię robienie małego refaktoryzacji, na pewno zajmie Ci to tylko minutę lub dwie. Każda taka bariera to zapach, który powinien skłonić do rozmowy. Zanotuj więc zniechęcenie i przedstaw je zespołowi. Przynajmniej należy to omówić podczas następnej retrospekcji.

Tam, gdzie pracuję, jest jedna praktyka programistyczna, która powoduje duże tarcie - Code Review (CR). Ilekroć zmieniam cokolwiek, co nie wchodzi w zakres mojego „zadania”, recenzenci napominają mnie, że utrudniam sprawdzenie zmiany. Jest to szczególnie prawdziwe, gdy bierze się pod uwagę refaktoryzację, ponieważ utrudnia to porównywanie różnic „linia po linii”. Podejście to jest tutaj standardem, co oznacza, że ​​oportunistyczne refaktoryzowanie jest rzadko wykonywane i tylko „planowane” refaktoryzowanie (które zwykle jest za mało, za późno) ma miejsce, jeśli w ogóle.

Twierdzę, że korzyści są tego warte i że 3 recenzentów będzie pracowało trochę ciężej (aby faktycznie zrozumieć kod przed i po, zamiast patrzeć na wąski zakres, w którym zmieniono linie - sama recenzja byłaby lepsza z tego powodu ), dzięki czemu skorzysta kolejnych 100 programistów czytających i utrzymujących kod. Kiedy przedstawiam ten argument moim recenzentom, mówią, że nie mają problemu z moim refaktoryzacją, o ile nie ma tego samego CR. Jednak twierdzę, że to mit:

(1) W większości przypadków zdajesz sobie sprawę, co i jak chcesz refaktoryzować, gdy jesteś w trakcie wykonywania zadania. Jak to ujął Martin Fowler:

Gdy dodajesz tę funkcjonalność, zdajesz sobie sprawę, że dodawany przez ciebie kod zawiera pewne powielanie z istniejącym kodem, więc musisz zmienić istniejący kod, aby oczyścić rzeczy ... Może coś działa, ale zdajesz sobie sprawę, że to będzie lepiej, jeśli interakcja z istniejącymi klasami zostanie zmieniona. Skorzystaj z okazji, aby to zrobić, zanim uznasz, że skończyłeś.

(2) Nikt nie będzie wyglądał przychylnie, gdy wydajesz „refaktoryzujące” CR, których nie powinieneś robić. CR ma pewne koszty ogólne, a twój kierownik nie chce, abyś „marnował czas” na refaktoryzację. Gdy jest dołączony do zmiany, którą należy wprowadzić, problem ten jest minimalizowany.

Problem jest zaostrzony przez Resharper, ponieważ każdy nowy plik, który dodam do zmiany (i nie wiem z góry, które dokładnie pliki zostaną zmienione) jest zwykle pełen błędów i sugestii - z których większość jest na miejscu i całkowicie zasługuje ustalenie.

Efektem końcowym jest to, że widzę okropny kod i po prostu go tam zostawiam. Jak na ironię, uważam, że poprawienie takiego kodu nie tylko nie poprawi mojej sytuacji, ale wręcz obniży je i pomaluje na mnie jako „nie skupionego” faceta, który marnuje czas na naprawianie rzeczy, na których nikogo nie obchodzi zamiast wykonywania swojej pracy. Czuję się z tym źle, ponieważ naprawdę gardzę złym kodem i nie mogę znieść oglądania go, nie mówiąc już o wywołaniu go z moich metod!

Wszelkie przemyślenia na temat tego, jak mogę zaradzić tej sytuacji?

OK, więc jest tu teraz więcej rzeczy, niż nadają się do komentarza.

tl; dr

Twoja intuicja dotycząca tego, co powinieneś zrobić (refaktoryzacja na bieżąco) jest prawidłowa.

Trudności w implementacji tego - biorąc pod uwagę, że musisz obejść słaby system sprawdzania kodu - sprowadzają się do trudności w manipulowaniu kodem źródłowym i VCS. Wiele osób powiedziało, że możesz i powinieneś podzielić swoje zmiany (tak, nawet w plikach) na wiele zatwierdzeń, ale wydaje się, że masz trudności z uwierzeniem w to.

Ty naprawdę można to zrobić. Właśnie to sugerujemy. Ty naprawdę powinien nauczyć się jak najwięcej o swoich narzędzi do edycji, manipulowania źródłem i kontroli wersji. Jeśli poświęcisz czas na nauczenie się, jak z nich dobrze korzystać, to znacznie ułatwi życie.

Problemy z przepływem pracy / polityką biurową

Zasadniczo podałbym tę samą rekomendację co GlenH7, aby utworzyć dwa zatwierdzenia - jedno z tylko refaktoryzacjami i (wyraźnie lub oczywiście) bez zmian funkcjonalnych i jedno z omawianymi zmianami funkcjonalnymi.

Może to być przydatne, choć, jeśli jesteś znalezienia wiele błędów, aby wybrać jedną kategorię błędu naprawić w ramach jednego CR. Następnie masz jedno zatwierdzenie z komentarzem typu „kod deduplikacji”, „napraw błędy typu X” lub cokolwiek innego. Ponieważ powoduje to jeden typ zmiany, prawdopodobnie w wielu miejscach, przeglądanie powinno być trywialne . Oznacza to, że nie możesz naprawić każdego znalezionego błędu, ale może sprawić, że przemoc będzie mniej bolesna.

Problemy z VCS

Podział zmian dokonanych w źródle roboczym na wiele zatwierdzeń nie powinien być wyzwaniem. Nie powiedziałeś, czego używasz, ale możliwe przepływy pracy to:

1. jeśli używasz git, masz do tego doskonałe narzędzia

   • możesz użyć git add -ido interaktywnego przemieszczania z wiersza poleceń
   • możesz używać git guii wybierać poszczególne porcje i linie na scenę (jest to jedno z niewielu narzędzi GUI związanych z VCS, które tak naprawdę wolę od wiersza poleceń, drugie to dobry 3-drożny edytor scalania)
   • możesz wykonać wiele drobnych zmian (pojedyncze zmiany lub naprawić tę samą klasę błędu w wielu miejscach), a następnie zmienić ich kolejność, scalić lub podzielić za pomocą rebase -i

2. jeśli nie używasz git, twój VCS może nadal mieć narzędzia do tego typu rzeczy, ale nie mogę doradzić, nie wiedząc, jakiego systemu używasz

   Wspomniałeś, że używasz TFS - który moim zdaniem jest kompatybilny z git od TFS2013. Warto spróbować poeksperymentować z użyciem lokalnego gitowego repozytorium repozytorium do pracy. Jeśli jest ono wyłączone lub nie działa dla ciebie, nadal możesz zaimportować źródło do lokalnego repozytorium git, pracować tam i używać go do eksportuj swoje ostateczne zatwierdzenia.

3. możesz to zrobić ręcznie w dowolnym VCS, jeśli masz dostęp do podstawowych narzędzi, takich jak diffi patch. To jest bardziej bolesne, ale z pewnością możliwe. Podstawowym przepływem pracy byłoby:

   1. wprowadź wszystkie zmiany, przetestuj itp.
   2. służy diffdo tworzenia (w kontekście lub ujednoliconego) pliku łatki ze wszystkimi zmianami od ostatniego zatwierdzenia
   3. podziel go na dwa pliki: skończysz na jednym pliku łatki zawierającym refaktoryzacje i jednym ze zmianami funkcjonalnymi
      • nie jest to całkowicie trywialne - narzędzia takie jak emacs diff mogą pomóc
   4. cofnij wszystko
   5. powróć do ostatniego zatwierdzenia, użyj, patchaby odtworzyć jeden z plików łatki, zatwierdzić te zmiany
      • NB. jeśli łatka nie zastosowała się czysto, może być konieczne ręczne naprawienie nieudanych kawałków
   6. powtórz 5 dla drugiego pliku poprawki

Teraz masz dwa zatwierdzenia, a zmiany odpowiednio podzielone na partycje.

Uwaga: prawdopodobnie istnieją narzędzia ułatwiające zarządzanie łatkami - po prostu ich nie używam, ponieważ git robi to za mnie.

Zakładam, że prośby o zmianę są duże i formalne w twojej firmie. Jeśli nie, po prostu wprowadź zmiany (jeśli to możliwe) w wiele małych zatwierdzeń (tak jak powinieneś).

Wszelkie przemyślenia na temat tego, jak mogę zaradzić tej sytuacji?

Kontynuować robienie tego, co robisz?

Mam na myśli, że wszystkie twoje myśli i wnioski są całkowicie poprawne. Powinieneś naprawić rzeczy, które widzisz. Ludzie nie planują wystarczająco refaktoryzacji. Ta korzyść dla całej grupy jest ważniejsza niż niewygodne dla niektórych.

Pomóc może być mniej wojowniczy. Przeglądy kodu nie powinny być bojowe: „Dlaczego to zrobiłeś?”, Powinny być oparte na współpracy „Cześć chłopaki, kiedy tu byłem, naprawiłem wszystkie te rzeczy!”. Praca (ze swoim kierownikiem / menedżerem, jeśli to możliwe), aby zmienić tę kulturę, jest trudna, ale bardzo ważna, aby stworzyć dobrze działający zespół programistów.

Możesz także współpracować (jeśli to możliwe z przełożonym / kierownikiem), aby zwiększyć znaczenie tych pomysłów z kolegami. Zadaj sobie pytanie, dlaczego nie zależy ci na jakości? zamiast pytać „dlaczego zawsze robisz te bezużyteczne rzeczy?”.

Mam wiele współczucia dla twojej sytuacji, ale niewiele konkretnych sugestii. Jeśli nic więcej, może przekonam cię, że choć sytuacja jest zła, może być gorzej. ZAWSZE może być gorzej. :-)

Po pierwsze, myślę, że masz (przynajmniej) dwa problemy ze swoją kulturą, a nie tylko jeden. Jednym z problemów jest podejście do refaktoryzacji, ale przeglądy kodu wydają się być odrębnym problemem. Spróbuję oddzielić moje myśli.

Recenzje kodu

Byłem w grupie, która HATED recenzuje kod. Grupa została utworzona przez połączenie dwóch grup z oddzielnych części firmy. Pochodzę z grupy, która od kilku lat robi recenzje kodu, z ogólnie dobrymi wynikami. Większość z nas uważa, że ​​recenzje kodu dobrze wykorzystują nasz czas. Połączyliśmy się w większą grupę i tak blisko, jak mogliśmy stwierdzić, ta „inna” grupa nigdy nie słyszała o recenzjach kodu. Teraz wszyscy pracowaliśmy nad „ich” bazą kodu.

Kiedy się połączyliśmy, nie było dobrze. Nowe funkcje były spóźnione 6-12 miesięcy, rok po roku. Zaległości w zakresie błędów były ogromne, rosnące i wyczerpujące życie. Własność kodu była silna, szczególnie wśród najstarszych „guru”. „Oddziały fabularne” czasami trwały lata i obejmowały kilka wydań; czasami NIKT, ale jeden deweloper widział kod, zanim trafił do głównej gałęzi. W rzeczywistości „gałąź funkcji” wprowadza w błąd, ponieważ sugeruje, że kod był gdzieś w repozytorium. Częściej było to tylko w indywidualnym systemie dewelopera.

Kierownictwo zgodziło się, że musimy „coś zrobić”, zanim jakość stanie się niedopuszczalnie niska. :-) Ich odpowiedzią były recenzje kodowe. Recenzje Code stały się oficjalnym przedmiotem „Thou Shalt”, który ma poprzedzać każdą odprawę. Narzędziem, którego użyliśmy, była komisja rewizyjna.

Jak to działało w kulturze, którą opisałem? Lepsze niż nic, ale było to bolesne i zajęło ponad rok osiągnięcie pewnego rodzaju minimalnego poziomu zgodności. Niektóre rzeczy, które zaobserwowaliśmy:

1. Narzędzie, którego używasz, zwykle skupia się na recenzowaniu kodu w określony sposób. To może być problem. Tablica recenzji zapewnia ładne, kolorowe różnice między wierszami i umożliwia dołączanie komentarzy do wiersza. To powoduje, że większość programistów ignoruje wszystkie wiersze, które się nie zmieniły. To dobrze w przypadku małych, izolowanych zmian. Nie jest tak dobry w przypadku dużych zmian, dużych fragmentów nowego kodu lub kodu zawierającego 500 wystąpień funkcji o zmienionej nazwie zmieszanej z 10 liniami nowej funkcjonalności.
2. Mimo że byliśmy w starej, chorej bazie kodu, która w większości nigdy wcześniej nie była recenzowana, stało się „niegrzeczne” dla recenzenta komentować wszystko, co NIE było linią zmian, nawet w celu wskazania oczywistego błędu. „Nie zawracaj mi głowy, to ważna odprawa i nie mam czasu na naprawianie błędów”.
3. Kup „łatwego” recenzenta. Niektórzy ludzie będą oglądać recenzję 10 plików z 2000 zmienionymi wierszami przez 3 minuty i klikną „Wyślij to!”. Wszyscy szybko dowiadują się, kim są ci ludzie. Jeśli naprawdę nie chcesz, aby Twój kod był sprawdzany w pierwszej kolejności, wyślij go do „łatwego” recenzenta. Zameldowanie nie zostanie spowolnione. Możesz odwdzięczyć się, stając się „łatwym” recenzentem jego kodu.
4. Jeśli nie znosisz recenzji kodu, po prostu zignoruj ​​wiadomości e-mail otrzymywane z komisji rewizyjnej. Zignoruj ​​obserwacje członków zespołu. Przez tygodnie. Dopóki nie będziesz mieć w kolejce 3 tuzinów otwartych recenzji, a twoje nazwisko pojawi się kilka razy na spotkaniach grupowych. Następnie zostań „łatwym” recenzentem i wyczyść wszystkie swoje recenzje przed obiadem.
5. Unikaj wysyłania kodu do „twardego” recenzenta. (Rodzaj programisty, który zadaje takie pytanie lub odpowiada na nie.) Wszyscy szybko dowiadują się, kim są „twarde” recenzenci, tak jak uczą się prostych. Jeśli recenzje kodu są stratą czasu (™), to przeczytanie szczegółowej informacji zwrotnej o kodzie, który posiadasz, jest zarówno stratą czasu (™), jak i obelgą.
6. Kiedy recenzje kodu są bolesne, ludzie odkładają je i piszą więcej kodu. Otrzymujesz mniej recenzji kodu, ale każda jest DUŻA. Potrzebujesz więcej, mniejszych recenzji kodu, co oznacza, że ​​zespół musi dowiedzieć się, jak uczynić je tak bezbolesnymi, jak to możliwe.

Myślałem, że zamierzam napisać kilka akapitów o Code Review, ale okazuje się, że pisałem głównie o kulturze. Myślę, że sprowadza się to do tego: recenzje kodu mogą być dobre dla projektu, ale zespół otrzymuje tylko te korzyści, na które zasługuje.

Refaktoryzacja

Czy moja grupa nie znosi refaktoryzacji bardziej niż nienawidzi recenzji kodu? Oczywiście! Z wszystkich oczywistych powodów, które zostały już wspomniane. Marnujesz mój czas na nieprzyjemny przegląd kodu, a nawet nie dodajesz funkcji ani nie naprawiasz błędu!

Ale kod wciąż desperacko wymagał refaktoryzacji. Jak postępować?

1. Nigdy nie mieszaj zmiany refaktoryzacji ze zmianą funkcjonalną. Wiele osób wspomniało o tym punkcie. Jeśli recenzje kodu są punktem tarcia, nie zwiększaj tarcia. To więcej pracy, ale powinieneś zaplanować osobną recenzję i osobne zameldowanie.
2. Zacznij od małego. Bardzo mały. Jeszcze mniejszy. Używaj bardzo małych refaktoryzacji, aby stopniowo uczyć ludzi, że refaktoryzacja i recenzje kodu nie są czystym złem. Jeśli uda ci się przekraść w jednym malutkim refaktoryzacji tygodniowo bez zbytniego bólu, za kilka miesięcy będziesz w stanie uciec od dwóch tygodniowo. I mogą być trochę większe. Lepsze niż nic.
3. Nie mieliśmy zasadniczo żadnych testów jednostkowych. Więc refaktoryzacja jest zabroniona, prawda? Niekoniecznie. W przypadku niektórych zmian kompilator jest testem jednostkowym. Skoncentruj się na refaktoryzacjach, które możesz przetestować. Unikaj zmian, jeśli nie można ich przetestować. Może zamiast tego spędzić czas na pisaniu testów jednostkowych.
4. Niektórzy programiści boją się refaktoryzacji, ponieważ boją się WSZYSTKICH zmian kodu. Rozpoznanie tego typu zajęło mi dużo czasu. Napisz fragment kodu, baw się nim, aż „zadziała”, a NIGDY nie chcesz go zmieniać. Nie muszą rozumieć DLACZEGO to działa. Zmiana jest ryzykowna. Nie ufają sobie, że dokonają JAKICHKOLWIEK zmian, a na pewno nie ufają Tobie. Refaktoryzacja powinna dotyczyć małych, bezpiecznych zmian, które nie zmieniają zachowania. Są programiści, dla których sam pomysł „zmiany, która nie zmienia zachowania” jest nie do pomyślenia . Nie wiem, co zasugerować w tych przypadkach. Myślę, że musisz spróbować pracować w obszarach, na których im nie zależy. Byłem zaskoczony, gdy dowiedziałem się, że ten typ może mieć długi,

Dlaczego nie robisz obu, ale z osobnymi zatwierdzeniami?

Twoi rówieśnicy mają rację. Przegląd kodu powinien oceniać kod, który został zmieniony przez kogoś innego. Nie powinieneś dotykać kodu, który recenzujesz dla kogoś innego, ponieważ wpływa to na twoją rolę recenzenta.

Ale jeśli zauważysz wiele rażących problemów, możesz skorzystać z dwóch opcji.

1. Jeśli sprawdzenie kodu było w przeciwnym razie w porządku, pozwól, aby część, którą sprawdziłeś, została zatwierdzona, a następnie refaktoryzuj kod w ramach drugiej odprawy.

2. Jeśli podczas przeglądu kodu wystąpiły problemy wymagające korekty, poproś programistę o refaktoryzację w oparciu o zalecenia Resharper.

Osobiście nienawidzę pomysłu publikowania recenzji kodów. Przegląd kodu powinien nastąpić po wprowadzeniu zmian w kodzie. Oczywiście mówię tu o programowaniu par. Po sparowaniu otrzymasz recenzję za darmo i lepszą recenzję kodu. Teraz wyrażam tutaj swoją opinię, wiem, że inni to podzielają, prawdopodobnie istnieją badania, które to potwierdzają.

Jeśli sprawisz, że recenzenci kodu sparują się z tobą, element wojenny przeglądu kodu powinien wyparować. Kiedy zaczynasz wprowadzać zmiany, których nie rozumiesz, pytanie można postawić w punkcie zmiany, omówić i zbadać alternatywne rozwiązania, które mogą prowadzić do lepszych refaktoryzacji. Otrzymasz przegląd kodu o wyższej jakości, ponieważ para będzie w stanie zrozumieć szerszy zakres zmiany i nie skupiać się tak bardzo na zmianach linia po linii, co otrzymujesz przy porównywaniu kodu obok siebie.

Oczywiście nie jest to bezpośrednia odpowiedź na problem refaktoryzacji poza zakresem prac nad zmianą, ale oczekiwałbym, że twoi rówieśnicy lepiej zrozumieliby uzasadnienie zmian, gdyby byli tam, kiedy wprowadziłeś zmianę.

Nawiasem mówiąc, zakładając, że robisz TDD lub jakąś formę czerwonego zielonego refaktora, jednym ze sposobów na zapewnienie zaangażowania od rówieśnika jest użycie techniki parowania ping ponga. Po prostu wyjaśniono, że sterownik jest obracany dla każdego etapu cyklu RGR, tj. Para 1 pisze test zakończony niepowodzeniem, para 2 naprawia go, para refaktory, para 2 zapisują test zakończony niepowodzeniem .... i tak dalej.

Prawdopodobnie ten problem odzwierciedla znacznie większy problem z kulturą organizacji. Wydaje się, że ludzie są bardziej zainteresowani wykonaniem „swojej pracy” właściwie niż ulepszeniem produktu, prawdopodobnie ta firma ma kulturę „winy” zamiast kultury współpracy, a ludzie wydają się bardziej zainteresowani ochroną siebie niż wizją całego produktu / firmy .

Moim zdaniem masz całkowitą rację, ludzie, którzy sprawdzają twój kod, są całkowicie w błędzie, jeśli mają skargi, ponieważ „dotykasz” rzeczy poza „twoim zadaniem”, próbujesz przekonać tych ludzi, ale nigdy nie sprzeciwiaj się twoim zasadom, dla mnie to jest najważniejsza jakość prawdziwego profesjonalisty.

A jeśli właściwa rzecz da ci złe liczby w jakiś głupi sposób korporacyjny, aby ocenić twoją pracę, jaki problem ?, kto chce „wygrać” tę grę ewaluacyjną w szalonej firmie ?, spróbuj ją zmienić !, a jeśli to niemożliwe lub jesteś zmęczony, znajdź inne miejsce, ale nigdy, nigdy nie sprzeciwiaj się swoim zasadom, to najlepsze, co masz.

Czasami refaktoryzacja jest złą rzeczą. Jednak nie z powodów podawanych przez recenzentów kodu; wygląda na to, że tak naprawdę nie dbają o jakość kodu, a Ty to obchodzi, co jest niesamowite. Ale są dwa powody, które powinny powstrzymać cię przed refaktoryzacją : 1. Nie możesz przetestować zmian dokonanych za pomocą testów automatycznych (testy jednostkowe lub cokolwiek innego) lub 2. Wprowadzasz zmiany w kodzie, którego nie rozumiesz zbyt dobrze dobrze; tzn. nie masz wiedzy specyficznej dla domeny, aby wiedzieć, jakie zmiany powinieneś wprowadzić.

1. Nie refaktoryzuj, gdy nie możesz przetestować zmian dokonanych za pomocą testów automatycznych.

Osoba dokonująca przeglądu kodu musi mieć pewność, że wprowadzone zmiany nie spowodowały uszkodzenia niczego, co działało wcześniej. Jest to największy powód, aby pozostawić działający kod w spokoju. Tak, zdecydowanie lepiej byłoby przeformułować tę funkcję o długości 1000 linii (to naprawdę obrzydliwość!) Na kilka funkcji, ale jeśli nie możesz zautomatyzować swoich testów, może być naprawdę trudno przekonać innych, że zrobiłeś wszystko dobrze. Zdecydowanie popełniłem już ten błąd.

Przed refaktoryzacją upewnij się, że istnieją testy jednostkowe. Jeśli nie ma testów jednostkowych, napisz trochę! Następnie dokonaj refaktoryzacji, a recenzenci kodu nie będą mieli (uzasadnionej) wymówki, by się denerwować.

Nie zmieniaj fragmentów kodu, które wymagają wiedzy specyficznej dla domeny, której nie masz.

Miejsce, w którym pracuję, zawiera dużo kodu wymagającego inżynierii chemicznej. Baza kodu używa idiomów wspólnych dla inżynierów chemików. Nigdy nie wprowadzaj zmian idiomatycznych dla pola. Może się wydawać, że to świetny pomysł, aby zmienić nazwę zmiennej o nazwie xdo molarConcentration, ale wiecie co? We wszystkich tekstach chemicznych stężenie molowe jest reprezentowane przez x. Zmiana nazwy powoduje, że trudniej jest wiedzieć, co właściwie dzieje się w kodzie dla osób w tej dziedzinie.

Zamiast zmieniać nazwy zmiennych idiomatycznych, po prostu wstaw komentarze wyjaśniające, czym one są. Jeśli są one nie idiomatyczne, zmień je. Nie należy pozostawiać i, j, k, x, y, itd krąży po lepsze nazwiska zadziała.

Ogólna zasada dotycząca skrótów: jeśli ludzie rozmawiają, używają skrótu, można używać tego skrótu w kodzie. Przykłady z bazy kodu w pracy:

Mamy następujące pojęcia, które zawsze skracamy, mówiąc o nich: „Obszar zainteresowania” zmienia się w „AOC”, „Eksplozja chmury pary” staje się VCE, coś w tym rodzaju. W naszej bazie kodu ktoś refaktoryzował wszystkie instancje zwane aoc do AreaOfConcern, VCE do vaporCloudExplosion, nPlanes do confiningPlanesCount ... co znacznie utrudniło odczytanie kodu osobom, które znały konkretną domenę. Byłem winny robienia takich rzeczy.

Może to wcale nie dotyczyć twojej sytuacji, ale są moje przemyślenia na ten temat.

To pytanie ma teraz dwa odrębne problemy - łatwość przeglądu zmian w recenzjach kodu i czas poświęcony na refaktoryzację.

Tam, gdzie pracuję, istnieje jedna praktyka programistyczna, która powoduje duże tarcie - Code Review (CR). Ilekroć zmieniam cokolwiek, co nie wchodzi w zakres mojego „zadania”, recenzenci napominają mnie, że utrudniam sprawdzenie zmiany.

Jak powiedzieli inne odpowiedzi - czy możesz oddzielić kontrole refaktoryzacji od kontroli zmiany kodu (niekoniecznie jako osobne recenzje)? W zależności od narzędzia, którego używasz do przeglądu kodu, powinieneś być w stanie zobaczyć różnice tylko między poszczególnymi wersjami (Atlassian's Crucible zdecydowanie to robi).

(2) Nikt nie będzie wyglądał przychylnie, gdy wydajesz „refaktoryzujące” CR, których nie powinieneś robić. CR ma pewne koszty ogólne, a twój kierownik nie chce, abyś „marnował czas” na refaktoryzację. Gdy jest dołączony do zmiany, którą należy wprowadzić, problem ten jest minimalizowany.

Jeśli refaktoryzacja jest prosta i sprawia, że ​​kod jest łatwiejszy do zrozumienia (co powinieneś zrobić, jeśli to tylko refaktoryzacja), wówczas weryfikacja kodu nie powinna zająć dużo czasu i powinna być minimalna narzut, który jest odzyskiwany w krótkim czasie, gdy ktoś musi przyjść i jeszcze raz przyjrzeć się kodowi. Jeśli twoi szefowie nie są w stanie tego zrozumieć, być może będziesz musiał delikatnie popchnąć ich do niektórych zasobów, które dyskutują, dlaczego reguła Boy Scout prowadzi do bardziej produktywnej relacji z twoim kodem. Jeśli uda ci się ich przekonać, że „marnowanie czasu” zaoszczędzi teraz dwa, pięć lub dziesięć razy więcej, gdy ty / ktoś inny wróci, aby wykonać więcej pracy nad tym kodem, problem powinien zniknąć.

Problem jest zaostrzony przez Resharper, ponieważ każdy nowy plik, który dodam do zmiany (i nie wiem z góry, które dokładnie pliki zostaną zmienione) jest zwykle pełen błędów i sugestii - z których większość jest na miejscu i zasługuje na to ustalenie.

Czy próbowałeś zwrócić uwagę tych problemów na kolegów i dyskutować, dlaczego warto je naprawić? I czy którykolwiek z nich może zostać naprawiony automatycznie (zakładając, że masz wystarczającą liczbę testów, aby potwierdzić, że nie zepsułeś rzeczy za pomocą automatycznego refaktoryzacji)? Czasami nie warto „refaktoryzować”, jeśli jest to naprawdę wybredne. Czy cały Twój zespół korzysta z ReSharper? Jeśli tak, czy masz wspólne zasady dotyczące tego, jakie ostrzeżenia / reguły są egzekwowane? Jeśli tak się nie stanie, być może powinieneś pokazać, gdzie narzędzie pomaga zidentyfikować obszary kodu, które są możliwymi źródłami przyszłego bólu.

Pamiętam mniej więcej 25 lat temu „oczyszczanie” kodu, nad którym akurat pracowałem do innych celów, głównie przez przeformatowanie bloków komentarzy i wyrównanie tabulatorów / kolumn w celu uporządkowania kodu, a zatem łatwiejszego do zrozumienia (bez faktycznych zmian funkcjonalnych) . Zdarza mi się podoba kod, który jest czysty i dobrze zorganizowany. Cóż, starsi programiści byli wściekli. Okazuje się, że użyli pewnego rodzaju porównania plików (diff), aby zobaczyć, co się zmieniło, w porównaniu z ich osobistymi kopiami pliku, a teraz daje to wszelkiego rodzaju fałszywe alarmy. Powinienem wspomnieć, że nasza biblioteka kodów znajdowała się na komputerze mainframe i nie miała kontroli wersji - po prostu nadpisałeś wszystko, co tam było, i to było to.

Morał tej historii? Nie wiem. Myślę, że czasami nie możesz zadowolić nikogo oprócz siebie. Nie marnowałem czasu (w moich oczach) - wyczyszczony kod był łatwiejszy do odczytania i zrozumienia. Po prostu prymitywne narzędzia kontroli zmian używane przez innych nakładają na nie dodatkową, jednorazową pracę. Narzędzia były zbyt prymitywne, aby zignorować spację / tabulacje i ponownie zamieszczać komentarze.

Jeśli możesz podzielić żądaną zmianę i niewymagany refaktor na (wiele) osobnych zatwierdzeń, jak zauważają @Useless, @Telastyn i inni, to jest to najlepsze, co możesz zrobić. Nadal będziesz pracować nad jednym CR, bez dodatkowych kosztów administracyjnych związanych z tworzeniem „refaktoryzacji”. Po prostu utrzymuj swoje zobowiązania małe i skoncentrowane.

Zamiast udzielać porad, jak to zrobić, wolę wskazać na znacznie większe wyjaśnienie (w rzeczywistości książkę) od specjalisty. W ten sposób będziesz mógł dowiedzieć się znacznie więcej. Przeczytaj Efektywna praca ze starszym kodem (autor: Michael Feathers) . Ta książka nauczy Cię, jak przeprowadzać refaktoryzację, przeplataną ze zmianami funkcjonalnymi.

Tematy obejmują:

• Zrozumienie mechanizmu zmiany oprogramowania: dodawanie funkcji, naprawianie błędów, ulepszanie projektu, optymalizacja wydajności
• Wprowadzanie starszego kodu do testowej uprzęży
• Pisanie testów, które chronią Cię przed wprowadzaniem nowych problemów
• Techniki, które można stosować w dowolnym języku lub platformie - z przykładami w Javie, C ++, C i C #
• Dokładne określenie, gdzie należy wprowadzić zmiany w kodzie
• Radzenie sobie ze starszymi systemami, które nie są obiektowe
• Obsługa aplikacji, które wydają się nie mieć żadnej struktury

Ta książka zawiera także katalog dwudziestu czterech technik przełamujących zależności, które pomagają pracować z elementami programu w izolacji i wprowadzać bezpieczniejsze zmiany.

Ja też jestem wielkim zwolennikiem zasady Boyscouta i oportunistycznego refaktoryzacji. Problem często polega na tym, że zarząd chce się zaangażować. Refaktoryzacja wiąże się z ryzykiem i kosztami. To, co zarząd często pomija, to także dług techniczny.

To ludzka natura. Jesteśmy przyzwyczajeni do radzenia sobie z prawdziwymi problemami, a nie próbowania im zapobiegać. Jesteśmy reaktywni, nie proaktywni.

Oprogramowanie jest niematerialne, dlatego trudno zrozumieć, że podobnie jak samochód wymaga serwisowania. Żadna rozsądna osoba nie kupiłaby samochodu i nigdy go nie serwisowała. Akceptujemy, że takie zaniedbanie skróciłoby jego żywotność, a na dłuższą metę byłoby bardziej kosztowne.

Pomimo tego, że wielu menedżerów to rozumie, ponoszą oni odpowiedzialność za zmiany w kodzie produkcyjnym. Istnieją polityki, które ułatwiają pozostawienie wystarczająco dobrze w spokoju. Często osoba, która potrzebuje przekonania, faktycznie przewyższa kierownika i po prostu nie chce walczyć o wprowadzenie w życie „świetnych pomysłów”.

Szczerze mówiąc, twój menedżer nie zawsze jest przekonany, że twoje lekarstwo jest tak wspaniałe, jak myślisz. (Olej węża?) Jest sprzedaż. Twoim zadaniem jest pomóc mu dostrzec korzyści.

Zarządzanie nie dzieli twoich priorytetów. Załóż kapelusz zarządzania i patrz oczami zarządzania. Bardziej prawdopodobne jest znalezienie odpowiedniego kąta. Bądź wytrwały. Spowodujesz więcej zmian, nie pozwalając, by pierwsza negatywna reakcja cię zniechęciła.

Jeśli możesz podzielić żądaną zmianę i niewymagany refaktor na dwa osobne żądania zmiany, jak zauważył @ GlenH7, to jest to najlepsze, co możesz zrobić. Nie jesteś więc „facetem, który marnuje czas”, ale „facetem, który pracuje dwa razy ciężej”.

Jeśli często znajdujesz się w sytuacji, w której nie możesz ich podzielić, ponieważ żądana praca wymaga teraz niekompletnego refaktora do skompilowania, sugerowałbym naleganie na posiadanie narzędzi do automatycznego sprawdzania standardów kodowania, przy użyciu argumentów przedstawionych tutaj (Resharper ostrzeżenia same w sobie mogą być dobrym kandydatem, nie znam go). Argumenty te są ważne, ale jest jeden dodatkowy, który można wykorzystać na swoją korzyść: mając te testy teraz uczynić go swoim obowiązek zdać testy na każdy popełnić.

Jeśli Twoja firma nie chce „marnować czasu na refaktoryzację” (zły znak z ich strony), powinna dostarczyć Ci plik „eliminacji ostrzeżeń” (każde narzędzie ma swój własny mechanizm), abyś już nie był zirytowany takie ostrzeżenia. Mówię to, aby zapewnić Ci opcje dla różnych scenariuszy, nawet w najgorszym przypadku. Lepiej jest też mieć jasno określone umowy między tobą a firmą, także dotyczące oczekiwanej jakości kodu, niż domniemane założenia z każdej strony.