Pogodzenie reguły skautowej i oportunistycznego refaktoryzacji z recenzjami kodu

55

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?

t0x1n
źródło
40
your manager doesn't want you to "waste your time" on refactoring
Czułbym
19
Oprócz posiadania wielu CR, kluczową kwestią jest to, że każde zatwierdzenie powinno mieć jeden cel: jeden dla refaktora, jeden dla wymagania / błędu / itp. W ten sposób przegląd może rozróżnić refaktor od żądanej zmiany kodu. Chciałbym również argumentować, że refaktor powinien być wykonany tylko wtedy, gdy istnieją testy jednostkowe, które dowodzą, że twój refaktor nic nie zepsuł (wujek Bob się zgadza).
2
@ t0x1n Nie widzę tego inaczej
Daenyth
2
@ t0x1n tak, tęskniłem za tym. Za mało kawy dziś rano. Z mojego doświadczenia wynika, że ​​istnieje kilka sposobów refaktoryzacji. Być może patrzysz na kod, który musisz zmodyfikować, i od razu wiesz, że wymaga on czyszczenia, więc najpierw to zrób. Może trzeba coś przefakturować, aby wprowadzić zmianę, ponieważ nowe wymaganie jest niezgodne z istniejącym kodem. Twierdzę, że refaktor jest nieodłącznie częścią twojej zmiany i nie powinien być uważany za osobny. Wreszcie, może widzisz, że kod jest do kitu w połowie zmiany, ale możesz go dokończyć. Refaktoryzuj po fakcie.
6
Punkt 1 nie twierdzi, że rozdzielenie zatwierdzeń jest niemożliwe. To po prostu sugeruje, że nie wiesz, jak to zrobić, lub twój VCS utrudnia. Robię to cały czas, nawet biorąc jeden zatwierdzenie i dzieląc go po fakcie.
Bezużyteczne

Odpowiedzi:

18

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.

Nieprzydatny
źródło
Nie jestem pewien, czy śledzę - czy punkt 3.3 zakłada, że ​​refaktoryzacja i zmiany funkcjonalne znajdują się w różnych plikach? W każdym razie nie. Być może rozdzielanie linii jest bardziej sensowne, ale nie sądzę, abyśmy mieli do tego odpowiednie narzędzia w naszym CVS (TFS). W każdym razie nie działałoby to w przypadku wielu (większości?) Refaktoryzacji, w których zmiany funkcjonalne polegają na zmianie refaktoryzowanej. Załóżmy na przykład, że zmieniłem metodę Foo (której muszę użyć w ramach mojej funkcji zmienionej), aby pobrać 3 parametry zamiast 2. Teraz kod funkcjonalny Imy opiera się na kodzie refaktora, nawet podział według linii nie pomoże.
t0x1n
1
Różne wiersze w tym samym pliku są poprawne w podanych przepływach pracy. Biorąc pod uwagę, że dwa zatwierdzenia byłyby sekwencyjne , jest całkowicie w porządku, aby drugi (funkcjonalny) zatwierdzenie zależał od pierwszego. Aha, a TFS2013 rzekomo obsługuje git.
Bezużyteczne
Używamy również TFS do kontroli źródła. Zakładasz, że drugie zatwierdzenie będzie funkcjonalne, podczas gdy zwykle byłoby odwrotnie (biorąc pod uwagę, że nie mogę z góry powiedzieć, jakie operacje refaktoryzacji należałoby zrobić). Przypuszczam, że mógłbym wykonać całą pracę związaną z funkcjonalnym + refaktoryzacją, a następnie pozbyć się wszystkiego funkcjonalnego i dodać go z powrotem w osobnym zatwierdzeniu. Mówię tylko, że jest to dużo kłopotów (i czasu), aby zadowolić kilku recenzentów. Bardziej rozsądnym podejściem jest dla mnie umożliwienie refaktoryzacji oportunistycznej, aw zamian wyrażenie zgody na takie zmiany samodzielnie (zaakceptowanie dodatkowej trudności).
t0x1n
3
Myślę, że naprawdę mnie nie rozumiesz. Edycja źródła i grupowanie zmian w zatwierdzeniach, tak, nawet zmiany w tym samym pliku, są logicznie oddzielnymi czynnościami. Jeśli wydaje się to trudne, wystarczy lepiej nauczyć się dostępnych narzędzi do zarządzania kodem źródłowym.
Bezużyteczne
1
Tak, twoje zrozumienie jest poprawne, miałbyś dwa kolejne zatwierdzenia z drugim (funkcjonalnym) w zależności od pierwszego (refaktoryzacja). Przepływ pracy różnic / łat opisany powyżej jest właśnie sposobem na zrobienie tego, który nie wymaga ręcznego usuwania zmian, a następnie ich ponownego zapisywania.
Bezużyteczne
29

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?”.

Telastyn
źródło
5
Tak, CR są duże i formalne. Zmiany są CRed, wylogowywane, a następnie przesyłane do kolejki. Małe zmiany powinny być dodawane jako iteracje do trwającego CR, a nie zatwierdzane osobno. WRT kontynuuje to, co robię, to może rzeczywiście przynieść korzyść grupie, ale obawiam się, że to nie przyniesie mi korzyści . Ci „niedogodności” to prawdopodobnie ci sami ludzie, którzy oceniliby mnie w corocznej recenzji. Problem ze zmianą kultury polega na tym, że wierzą w nią wielcy wodzowie. Być może muszę po prostu zdobyć więcej szacunku w ich oczach, zanim
spróbuję
13
@ t0x1n - Nie patrz na mnie. Zrobiłem karierę, robiąc właściwe rzeczy wobec ludzi, którzy uparcie patrzą na ssanie. Może nie byłbym tak opłacalny, jak bym mógł uszczęśliwić ludzi, ale śpię dobrze.
Telastyn
Dzięki za bycie szczerym. Rzeczywiście jest to coś do rozważenia.
t0x1n
1
Często też na to wpadam. Zapewnienie, że mam łatkę „czyszczącą”, a następnie łatkę roboczą, bardzo pomaga. Zwykle walczę w systemie, a potem wychodzę do pracy, która jest mniej stresująca. To powiedziawszy, czasami istnieją uzasadnione powody do obaw twoich współpracowników. Na przykład, jeśli kod szybko przechodzi do produkcji i nie ma wystarczających testów. Przegląd kodu widziałem jako próbę uniknięcia testowania. To nie działa. Przegląd kodu pomaga zachować jednolitość treści kodu. Robi niewiele w przypadku błędów.
Sean Perry
1
@SeanPerry zgodził się - ale mówię o normalnych okolicznościach, w których istnieją testy, zostaną wykonane pomyłki błędów itp.
t0x1n
14

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,
GraniteRobert
źródło
1
To bardzo przemyślana odpowiedź, dziękuję! W szczególności zgadzam się z tym, w jaki sposób narzędzie CR może wpłynąć na cel przeglądu ... linia po linii to łatwe wyjście, które nie wymaga faktycznego zrozumienia tego, co działo się wcześniej i co dzieje się teraz. I oczywiście kod, który się nie zmienił, jest idealny, nie trzeba nigdy na to patrzeć ...
t0x1n
1
Mogło być gorzej. Mogło padać”. Kiedy przeczytałem twój ostatni akapit (drugi punkt # 4), pomyślałem: potrzebują więcej recenzji, recenzji kodera . I trochę refaktoryzacji, jak w: „yourSalary = 0”
Absolutnie na wielu frontach, niesamowita odpowiedź. Całkowicie widzę, skąd pochodzisz: osobiście jestem w tej samej sytuacji i to jest niezwykle frustrujące. Ciągle walczysz o jakość i dobre praktyki, a wsparcie jest nie tylko ze strony kierownictwa, ale przede wszystkim innych programistów w zespole.
julealgon
13

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.


źródło
Z Twojej odpowiedzi wynika, że ​​wierzysz w silną własność kodu. Zachęcam do zapoznania się z przemyśleniami Fowlera o tym, dlaczego to zły pomysł: martinfowler.com/bliki/CodeOwnership.html . Konkretnie odnosząc się do twoich punktów: (1) jest mitem, ponieważ refaktoryzacja ma miejsce podczas dokonywania zmian - nie przed lub po osobnym, czystym, niezwiązanym ze sobą związku, tak jak wymagałoby to oddzielnych CR. (2) W przypadku większości deweloperów tak się nigdy nie stanie. Nigdy . Większość deweloperów nie dba o te rzeczy, a tym bardziej, gdy pochodzi od innego faceta, który nie jest ich menedżerem. Mają własne rzeczy, które chcą robić.
t0x1n
8
@ t0x1n: Jeśli twoi menedżerowie nie dbają o refaktoryzację, a twoi koledzy programiści nie dbają o refaktoryzację ... wtedy ta firma powoli tonie. Uciec! :)
logc
8
@ t0x1n tylko dlatego, że dokonujesz zmian razem, nie oznacza to, że musisz je zatwierdzić razem. Poza tym często przydatne jest sprawdzenie, czy refaktoryzacja nie miała nieoczekiwanych skutków ubocznych oddzielnie od sprawdzenia, czy zmiana funkcjonalna przyniosła oczekiwany efekt. Oczywiście może to nie dotyczyć wszystkich refaktoryzacji, ale ogólnie nie jest to zła rada.
Bezużyteczne
2
@ t0x1n - Nie pamiętam, żeby mówić o silnym posiadaniu kodu. Moja odpowiedź polegała na utrzymaniu czystości roli recenzenta. Jeśli recenzent wprowadza zmiany, nie są już tylko recenzentem.
3
@ GlenH7 Być może było tu jakieś nieporozumienie - nie jestem recenzentem. Po prostu koduję to, czego potrzebuję, i napotykam kod, który mogę ulepszyć w tym procesie. Moi recenzenci narzekają, kiedy to robię.
t0x1n
7

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.

żonkile
źródło
Doskonałe punkty. Niestety szczerze wątpię, czy będę w stanie zmienić „system”. Czasami recenzenci pochodzą również z różnych stref czasowych i lokalizacji geograficznych, więc w takich przypadkach nie poleci.
t0x1n
6

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.

AlfredoCasado
źródło
1
Zaczynasz chcieć wygrać grę ewaluacyjną, gdy zdasz sobie sprawę, że nagradza cię ona podwyżkami pensji, bonusami i opcjami na akcje :)
t0x1n
2
Nie. Wymieniasz pieniądze na zasady @ t0x1n. Proste. Masz trzy możliwości: naprawić system, zaakceptować system, opuścić system. Opcje 1 i 3 są dobre dla duszy.
Sean Perry
2
nie tylko jest szkodliwe dla duszy, firma z zasadami, które koncentrują się na lokalnych maksymach, jest zwykle mniej optymalna niż firma, która koncentruje się na globalnych maksymach. Nie tylko to, praca to nie tylko pieniądze, spędzasz dużo czasu każdego dnia w pracy, czujesz się komfortowo w swojej pracy, czujesz, że robisz właściwe rzeczy, prawdopodobnie znacznie lepiej niż kilka dolarów więcej każdego miesiąca.
AlfredoCasado
1
Meh, dusze są przereklamowane;)
t0x1n
2
@SeanPerry Naprawdę próbowałem naprawić system, ale było to bardzo trudne. (1) Byłem w tym praktycznie sam i ciężko jest mi przeciwstawić się wielkim wodzom (jestem zwykłym deweloperem, nawet wyższym). (2) Te rzeczy wymagają czasu, którego po prostu nie miałem. Jest dużo pracy, a całe środowisko jest bardzo czasochłonne (e-maile, przerwy, CR, nieudane cykle testowe, które musisz naprawić, spotkania itp.). Dokładam wszelkich starań, aby filtrować i być produktywnym, ale zazwyczaj ledwo mogę dokończyć „zalecaną” pracę na czas (wysokie standardy tu nie pomagają), nie mówiąc już o pracy nad zmianą systemu ...
t0x1n
5

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.

Cody Piersall
źródło
Dzięki Cody. Jeśli chodzi o „twoi recenzenci kodu nie będą mieli (uzasadnionego) usprawiedliwienia dla zdenerwowania” - ich usprawiedliwienie jest już bezprawne, ponieważ są zdenerwowani zwiększoną trudnością przejrzenia zmian, a nie poprawnością, czytelnością lub czymkolwiek podobnym.
t0x1n
2

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.

Chris Cooper
źródło
W kwestii separacji CR wskazałem w moim poście i kilku innych komentarzach, dlaczego uważam to za niemożliwe.
t0x1n
Nie mówię o wybrednych rzeczach, mówię o rzeczywistych problemach z wydajnością i poprawnością, a także o nadmiarowym kodzie, zduplikowanym kodzie itp. I to tylko R # rzeczy, jest też naprawdę zły kod, który mogę łatwo naprawić . Niestety nie wszyscy z mojego zespołu używają resharpera, a nawet ci, którzy nie traktują tego zbyt poważnie. Potrzebne są ogromne wysiłki edukacyjne i być może spróbuję poprowadzić coś takiego. Jest to trudne, ponieważ ledwo mam czas na pracę, którą muszę wykonać, nie mówiąc już o projektach edukacyjnych. Być może muszę po prostu zaczekać na wykorzystanie okresu przestoju.
t0x1n
Po prostu wejdę i powiem, że to zdecydowanie nie jest niemożliwe , ponieważ cały czas to robię. Dokonaj zmiany, którą wprowadziłbyś bez refaktoryzacji, sprawdź to, a następnie refaktoryzuj, aby posprzątać i sprawdzić. To nie jest nauka o rakietach. Aha, i bądź przygotowany na obronę, dlaczego uważasz, że warto poświęcić czas na refaktoryzację i przeglądanie refaktoryzowanego kodu.
Eric King,
@EricKing Chyba mógłbym to zrobić. Jednak: (1) musiałbym pracować z brzydkim kodem i notować, co chcę poprawić, dopóki nie skończę ze zmianami funkcjonalnymi, które są do bani i faktycznie spowalniają mój postęp funkcjonalny (2) Po przesłaniu zmian funkcjonalnych i ponownie przejrzę moje notatki do refaktoryzacji, będzie to tylko pierwsza iteracja, a zakończenie refaktoryzacji może zająć więcej czasu, co, jak zasugerowałeś, z trudem tłumaczyłbym moim menedżerom, ponieważ moja praca jest już „ukończona”.
t0x1n
2
„Mówię o rzeczywistych problemach z wydajnością i poprawnością” - może to rozciągać definicję refaktoryzacji; jeśli kod jest rzeczywiście niepoprawny, stanowiłoby to naprawę błędu. Jeśli chodzi o problemy z wydajnością, nie jest to coś, co powinno się naprawiać w ramach zmiany funkcji, to prawdopodobnie coś, co wymaga pomiaru, dokładnych testów i oddzielnej recenzji kodu.
Chris Cooper
2

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.

Phil Perry
źródło
Tak, dostaję się też za odstępy, a także trywialne rzeczy, takie jak zbędne rzucanie itp. Cokolwiek nie jest całkowicie wymagane przez moją zmianę, to „szum”.
t0x1n
2

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.

Hbas
źródło
2

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.

Mario T. Lanza
źródło
1

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.

logc
źródło
To byłaby dobra propozycja dla nowego projektu. Jednak nasza obecna baza kodów jest ogromna, a Resharper emituje wiele błędów dla większości plików. Po prostu jest za późno, aby go wymusić, a wyeliminowanie istniejących błędów jeszcze bardziej go pogorszy - będziesz tęsknił za nowym kodem. Jest też wiele błędów, problemów i zapachów, których statyczny analizator nie może wyłapać, podałem tylko ostrzeżenia Resharper jako przykład. Znowu mogłem nazwać „zbytnią część” zbyt surowo, powinienem powiedzieć coś w stylu „poświęconego czasu na coś, co nie jest priorytetem”.
t0x1n
@ t0x1n: reguła Boy Scout dotyczy głównie modułów, których dotykasz. Może to pomóc w narysowaniu pierwszej linii podziału. Wiem, że tłumienie ostrzeżeń nie jest dobrym pomysłem, ale z punktu widzenia firmy tłumienie ich w nowym kodzie jest poprawne i przestrzeganie ich konwencji - no cóż, może mnie
pociąga
1
to jest frustrujące! Dotykam tylko plików, które i tak bym dotknął w ramach mojego zadania, ale skargi są takie same! Ostrzeżenia, o których mówię, nie są ostrzeżeniem o stylu, mówię o rzeczywistych problemach z wydajnością i poprawnością, a także o nadmiarowym kodzie, zduplikowanym kodzie itp.
t0x1n
@ t0x1n: to naprawdę frustrujące. Pamiętaj, że nie chodziło mi tylko o „statyczną analizę kodu”, ale także o inne zalecenia, coś równoważnego Nexusowi. Oczywiście żadne narzędzie nie przechwytuje 100% semantyki; to tylko strategia naprawcza.
logc