Mój szef prosi mnie, abym przestał pisać małe funkcje i robił wszystko w tej samej pętli

Przeczytałem książkę Robert C. Martin o nazwie Clean Code . W tej książce widziałem wiele metod czyszczenia kodu, takich jak pisanie małych funkcji, ostrożne wybieranie nazw itp. Wydaje się, że jest to najbardziej interesująca książka o czystym kodzie, którą przeczytałem. Jednak dzisiaj mój szef nie polubił sposobu, w jaki napisałem kod po przeczytaniu tej książki.

Jego argumentami były

• Pisanie małych funkcji jest uciążliwe, ponieważ zmusza cię do przejścia do każdej małej funkcji, aby zobaczyć, co robi kod.
• Umieść wszystko w głównej dużej pętli, nawet jeśli główna pętla ma więcej niż 300 linii, jest szybszy do odczytania.
• Pisz małe funkcje tylko wtedy, gdy musisz powielić kod.
• Nie pisz funkcji o nazwie komentarza, umieść złożony wiersz kodu (3-4 wiersze) z komentarzem powyżej; podobnie możesz bezpośrednio zmodyfikować uszkodzony kod

Jest to sprzeczne ze wszystkim, co przeczytałem. Jak zwykle piszesz kod? Jedna główna duża pętla, żadnych małych funkcji?

Językiem, którego używam, jest głównie Javascript. Naprawdę mam teraz trudności z czytaniem, ponieważ usunąłem wszystkie moje małe, wyraźnie nazwane funkcje i umieściłem wszystko w dużej pętli. Jednak mój szef lubi to w ten sposób.

Jednym z przykładów było:

// The way I would write it
if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

// The way he would write it
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

W książce, którą przeczytałem, na przykład komentarze są uważane za brak napisania czystego kodu, ponieważ są one przestarzałe, jeśli piszesz małe funkcje i często prowadzą do nieaktualizowanych komentarzy (modyfikujesz kod, a nie komentarz). Jednak usuwam komentarz i piszę funkcję o nazwie komentarza.

Cóż, chciałbym uzyskać porady, w jaki sposób / ćwiczyć lepiej pisać czysty kod?

Najpierw biorąc przykłady kodu. Faworyzujesz:

if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Twój szef napisałby to jako:

// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

Moim zdaniem oba mają problemy. Gdy czytam twój kod, od razu pomyślałem: „możesz zastąpić ifto wyrażeniem potrójnym”. Potem przeczytałem kod twojego szefa i pomyślałem „dlaczego zastąpił twoją funkcję komentarzem?”.

Sugerowałbym, że optymalny kod jest między nimi:

phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

To daje najlepsze z obu światów: uproszczone wyrażenie testowe, a komentarz zostaje zastąpiony testowanym kodem.

Jednak jeśli chodzi o poglądy szefa na projekt kodu:

Pisanie małych funkcji jest uciążliwe, ponieważ zmusza cię do przejścia do każdej małej funkcji, aby zobaczyć, co robi kod.

Jeśli funkcja ma dobrą nazwę, tak nie jest. isApplicationInProductionjest oczywiste i nie powinno być konieczne sprawdzanie kodu, aby zobaczyć, co robi. W rzeczywistości jest odwrotnie: sprawdzenie kodu ujawnia mniej intencji niż nazwa funkcji (dlatego szef musi uciekać się do komentarzy).

Umieść wszystko w głównej dużej pętli, nawet jeśli główna pętla ma więcej niż 300 linii, jest szybszy do odczytania

Skanowanie może być szybsze, ale aby naprawdę „odczytać” kod, musisz być w stanie skutecznie wykonać go w swojej głowie. Jest to łatwe z małymi funkcjami i jest naprawdę bardzo trudne z metodami o długości 100 linii.

Napisz tylko małe funkcje, jeśli musisz powielić kod

Nie zgadzam się. Jak pokazuje twój przykład kodu, małe, dobrze nazwane funkcje poprawiają czytelność kodu i powinny być używane, gdy np. Nie jesteś zainteresowany „jak”, a jedynie „co” części funkcjonalności.

Nie pisz funkcji o nazwie komentarza, umieść złożony wiersz kodu (3-4 wiersze) z komentarzem powyżej. W ten sposób możesz bezpośrednio zmodyfikować uszkodzony kod

Naprawdę nie rozumiem uzasadnienia tego, zakładając, że to naprawdę poważne. Tego rodzaju spodziewam się po napisaniu w parodii na twitterowym koncie The Expert Beginner . Komentarze mają podstawową wadę: nie są kompilowane / interpretowane, więc nie mogą być testowane jednostkowo. Kod zostaje zmodyfikowany, a komentarz zostaje sam, a ty nie wiesz, co jest poprawne.

Pisanie samodokumentującego się kodu jest trudne i czasem potrzebne są dodatkowe dokumenty (nawet w formie komentarzy). Jednak pogląd „wuja Boba”, że komentarze są błędem kodowania, jest zbyt często aktualny.

Poproś szefa, aby przeczytał książkę Czysty kod i postaraj się powstrzymać, aby twój kod był mniej czytelny, aby go zadowolić. Ostatecznie jednak, jeśli nie możesz go przekonać do zmiany, musisz albo stanąć w kolejce, albo znaleźć nowego bossa, który lepiej koduje.

Istnieją inne problemy

Żaden kod nie jest dobry, ponieważ oba w zasadzie nadmuchują kod w przypadku testowym debugowania . Co jeśli chcesz przetestować więcej rzeczy z jakiegokolwiek powodu?

phoneNumber = DEV_PHONE_NUMBER_WHICH_CAUSED_PROBLEMS_FOR_CUSTOMERS;

lub

phoneNumber = DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY;

Czy chcesz dodać jeszcze więcej oddziałów?

Istotnym problemem jest to, że w zasadzie kopiujesz część kodu, a zatem nie testujesz prawdziwego kodu. Piszesz kod debugowania, aby przetestować kod debugowania, ale nie kod produkcyjny. To tak, jakby częściowo stworzyć równoległą bazę kodu.

Kłócisz się z szefem o to, jak sprytniej pisać zły kod. Zamiast tego powinieneś naprawić nieodłączny problem samego kodu.

Zastrzyk zależności

Tak powinien wyglądać Twój kod:

phoneNumber = headers.resourceId;

Nie ma tutaj rozgałęzień, ponieważ logika tutaj nie ma rozgałęzień. Program powinien pobrać numer telefonu z nagłówka. Kropka.

Jeśli chcesz mieć DEV_PHONE_NUMBER_FROM_OTHER_COUNTRYw rezultacie, powinieneś to włożyć headers.resourceId. Jednym ze sposobów na to jest po prostu wstrzyknięcie innego headersobiektu dla przypadków testowych (przepraszam, jeśli to nie jest poprawny kod, moje umiejętności JavaScript są nieco zardzewiałe):

function foo(headers){
    phoneNumber = headers.resourceId;
}

// Creating the test case
foo({resourceId: DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY});

Zakładając, że headersjest to część odpowiedzi otrzymanej od serwera: idealnie, jeśli masz cały serwer testowy, który dostarcza headersróżnego rodzaju narzędzia do celów testowych. W ten sposób testujesz aktualny kod produkcyjny w stanie, w jakim jest, a nie jakiś w połowie zduplikowany kod, który może, ale nie musi, działać jak kod produkcyjny.

Nie ma na to „właściwej” lub „złej” odpowiedzi. Przedstawię jednak swoją opinię na podstawie 36-letniego doświadczenia zawodowego w projektowaniu i rozwijaniu systemów oprogramowania ...

1. Nie ma czegoś takiego jak „samodokumentujący się kod”. Dlaczego? Ponieważ to twierdzenie jest całkowicie subiektywne.
2. Komentarze nigdy nie są błędami. To, co jest awarią, to kod, którego w ogóle nie można zrozumieć bez komentarzy.
3. 300 nieprzerwanych linii kodu w jednym bloku kodu to koszmar konserwacji i bardzo podatny na błędy. Taki blok silnie wskazuje na zły projekt i planowanie.

Mówiąc bezpośrednio do przykładu, który podałeś ... Umieszczenie isApplicationInProduction()we własnej rutynie to mądra (er) rzecz do zrobienia. Dzisiaj ten test jest po prostu sprawdzeniem „nagłówków” i może być obsługiwany przez ?:operator trójskładnikowy ( ). Jutro test może być znacznie bardziej złożony. Ponadto „headers.resourceId” nie ma wyraźnego związku z „w statusie produkcyjnym” aplikacji; Uważam, że test dla takiego stanu musi być oddzielone od danych bazowych; podprogram zrobi to, a trójka tego nie zrobi. Dodatkowo pomocny komentarz wyjaśnia, dlaczego resourceId jest testem „produkcyjnym”.

Uważaj, aby nie przesadzić z „małymi, wyraźnie nazwanymi funkcjami”. Procedura powinna zawierać pomysł bardziej niż „tylko kod”. Popieram sugestię Amona phoneNumber = getPhoneNumber(headers)i dodaję, że getPhoneNumber()powinien wykonać test „statusu produkcyjnego”isApplicationInProduction()

„Istot nie wolno pomnażać poza koniecznością”.

- Brzytwa Ockhama

Kod musi być tak prosty, jak to możliwe. Błędy lubią się ukrywać między złożonością, ponieważ trudno je tam zauważyć. Co sprawia, że ​​kod jest prosty?

Małe jednostki (pliki, funkcje, klasy) są dobrym pomysłem . Małe jednostki są łatwe do zrozumienia, ponieważ jest mniej rzeczy, które musisz zrozumieć od razu. Normalni ludzie mogą żonglować tylko ~ 7 koncepcjami naraz. Ale rozmiar nie jest mierzony tylko w liniach kodu . Mogę napisać jak najmniej kodu, „grając w golfa” (wybierając krótkie nazwy zmiennych, biorąc „sprytne” skróty, rozbijając jak najwięcej kodu na jedną linię), ale efekt końcowy nie jest prosty. Próba zrozumienia takiego kodu jest bardziej jak inżynieria odwrotna niż czytanie.

Jednym ze sposobów skrócenia funkcji jest wyodrębnienie różnych funkcji pomocniczych. To może być dobry pomysł, gdy wydobywa samodzielny element złożoności . W oderwaniu od tego, złożoność jest znacznie łatwiejsza do zarządzania (i testowania!) Niż w przypadku niepowiązanego problemu.

Ale każde wywołanie funkcji ma narzut poznawczy : nie muszę tylko rozumieć kodu w ramach mojego obecnego fragmentu kodu, muszę także rozumieć, w jaki sposób współdziała on z kodem na zewnątrz . Myślę, że można śmiało powiedzieć, że wyodrębniona funkcja wprowadza do niej większą złożoność niż wyodrębnia . To twój szef rozumiał przez „ małe funkcje [są] bólem, ponieważ zmusza cię do przejścia do każdej małej funkcji, aby zobaczyć, co robi kod. ”

Czasami długie nudne funkcje mogą być niezwykle łatwe do zrozumienia, nawet jeśli mają setki linii długości. Dzieje się tak zwykle w przypadku kodu inicjalizacji i konfiguracji, np. Podczas ręcznego tworzenia GUI bez edytora przeciągania i upuszczania. Nie ma samodzielnego elementu złożoności, który można rozsądnie wydobyć. Ale jeśli formatowanie jest czytelne i pojawiają się komentarze, naprawdę nie jest trudno śledzić, co się dzieje.

Istnieje wiele innych wskaźników złożoności: liczba zmiennych w zakresie powinna być jak najmniejsza. To nie znaczy, że powinniśmy unikać zmiennych. Oznacza to, że powinniśmy ograniczyć każdą zmienną do możliwie najmniejszego zakresu tam, gdzie jest ona potrzebna. Zmienne stają się również prostsze, jeśli nigdy nie zmienimy ich wartości.

Bardzo ważnym miernikiem jest złożoność cykliczna (złożoność McCabe). Mierzy liczbę niezależnych ścieżek za pomocą fragmentu kodu. Liczba ta rośnie wykładniczo z każdym warunkiem. Każda warunkowa lub pętla podwaja liczbę ścieżek. Istnieją dowody sugerujące, że wynik powyżej 10 punktów jest zbyt złożony. Oznacza to, że bardzo długa funkcja, która może mieć wynik 5, może być lepsza niż bardzo krótka i gęsta funkcja z wynikiem 25. Możemy zmniejszyć złożoność, wyodrębniając przepływ kontrolny do oddzielnych funkcji.

Warunek jest przykładem złożoności, którą można całkowicie wyodrębnić:

function bigFatFunction(...) {
  ...
  phoneNumber = getPhoneNumber(headers);
  ...
}

...

function getPhoneNumber(headers) {
  return headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;
}

Nadal jest to bardzo przydatne. Nie jestem pewien, czy to znacznie zmniejsza złożoność, ponieważ ten warunek nie jest bardzo warunkowy . W produkcji zawsze będzie podążać tą samą ścieżką.

Złożoność nigdy nie może zniknąć. Można go tylko przetasować. Czy wiele małych rzeczy jest prostszych niż kilka dużych rzeczy? To zależy bardzo od okoliczności. Zwykle jest taka kombinacja, która wydaje się odpowiednia. Znalezienie kompromisu między różnymi czynnikami złożoności wymaga intuicji i doświadczenia oraz odrobiny szczęścia.

Umiejętność pisania bardzo małych funkcji i bardzo prostych funkcji jest przydatną umiejętnością, ponieważ nie można dokonać wyboru bez znajomości alternatyw. Ślepe przestrzeganie zasad lub najlepszych praktyk bez zastanawiania się, w jaki sposób odnoszą się one do obecnej sytuacji, prowadzi w najlepszym razie do średnich wyników, w najgorszym przypadku do programów kultowych .

Nie zgadzam się z twoim szefem. Jego argumenty nie są nieważne, ale nie jest też błędna książka Clean Code. Prawdopodobnie lepiej jest postępować zgodnie z wytycznymi szefa, ale sam fakt, że myślisz o tych problemach, próbując znaleźć lepszy sposób, jest bardzo obiecujący. W miarę zdobywania doświadczenia łatwiej będzie znaleźć dobry faktoring dla swojego kodu.

(Uwaga: ta odpowiedź oparta jest w części na przemyśleniach z posta na blogu Reasonable Code na The Whiteboard autorstwa Jimmy Hoffa , który zapewnia ogólny widok tego, co czyni kod prostym.)

Styl programowania Roberta Martina jest polaryzujący. Znajdziesz wielu programistów, nawet doświadczonych, którzy znajdą wiele wymówek, dlaczego dzielenie „tyle” to zbyt wiele, a utrzymywanie nieco większych funkcji to „lepszy sposób”. Jednak większość tych „argumentów” jest często wyrazem niechęci do zmiany starych nawyków i uczenia się czegoś nowego.

Nie słuchaj ich!

Ilekroć możesz zapisać komentarz, refaktoryzując fragment kodu do osobnej funkcji z wyrazistą nazwą, zrób to - najprawdopodobniej poprawi on twój kod. Nie posunąłeś się tak daleko, jak robi to Bob Martin w swojej czystej książce kodów, ale ogromna większość kodu, który widziałem w przeszłości, który powodował problemy z konserwacją, zawierała zbyt duże funkcje, a nie zbyt małe. Dlatego powinieneś spróbować napisać mniejsze funkcje z samoopisującymi się nazwami.

Automatyczne narzędzia do refaktoryzacji pozwalają łatwo, łatwo i bezpiecznie wyodrębnić metody. I proszę, nie bierz na poważnie ludzi, którzy zalecają pisanie funkcji z> 300 liniami - tacy ludzie zdecydowanie nie są wykwalifikowani, aby powiedzieć ci, jak powinieneś pisać.

W twoim przypadku: chcesz numer telefonu. Albo jest oczywiste, jak uzyskać numer telefonu, a następnie piszesz oczywisty kod. Lub nie jest oczywiste, jak uzyskać numer telefonu, a następnie piszesz dla niego metodę.

W twoim przypadku nie jest oczywiste, jak uzyskać numer telefonu, więc piszesz dla niego metodę. Wdrożenie nie jest oczywiste, ale dlatego umieszczasz je w osobnej metodzie, więc musisz poradzić sobie z tym tylko raz. Przydałby się komentarz, ponieważ implementacja nie jest oczywista.

Metoda „isApplicationInProduction” jest całkowicie bezcelowa. Wywołanie go z metody getPhonenumber nie sprawia, że ​​implementacja staje się bardziej oczywista, a jedynie utrudnia ustalenie, co się dzieje.

Nie pisz małych funkcji. Pisz funkcje, które mają ściśle określony cel i spełniają ten dobrze określony cel.

PS. Realizacja wcale mi się nie podoba. Zakłada, że ​​brak numeru telefonu oznacza, że ​​jest to wersja deweloperska. Jeśli więc numer telefonu jest nieobecny w produkcji, nie tylko nie obsługujesz go, ale zastępujesz losowy numer telefonu. Wyobraź sobie, że masz 10 000 klientów, a 17 nie ma numeru telefonu i masz kłopoty z produkcją. To, czy jesteś w produkcji, czy w fazie rozwoju, powinno być sprawdzane bezpośrednio, a nie na podstawie czegoś innego.

Nawet ignorując fakt, że żadna implementacja nie jest aż tak dobra, chciałbym zauważyć, że jest to zasadniczo kwestia gustu przynajmniej na poziomie wyodrębnienia trywialnych funkcji jednorazowego użytku.

W większości przypadków liczba wierszy nie jest użyteczną miarą.

300 (a nawet 3000) wierszy całkowicie trywialnego, czysto sekwencyjnego kodu (Setup, lub coś takiego) rzadko stanowi problem (ale może być lepiej wygenerowany automatycznie lub jako tabela danych lub coś takiego), 100 linii zagnieżdżonych pętli z mnóstwem skomplikowanych warunki wyjścia i matematykę, jakie można znaleźć w Eliminacji Gaussa lub odwróceniu macierzy, lub takie mogą być o wiele za dużo, aby można było z łatwością je wykonać.

Dla mnie nie napisałbym funkcji jednorazowego użytku, chyba że ilość kodu potrzebna do zadeklarowania rzeczy byłaby znacznie mniejsza niż ilość kodu tworzącego implementację (chyba że miałbym powód, żeby powiedzieć, że chcę łatwo wstrzyknąć błąd). Jeden warunek rzadko pasuje do tego rachunku.

Teraz pochodzę z małego, wbudowanego świata, w którym musimy również wziąć pod uwagę takie rzeczy, jak głębokość stosu i koszty wywołania / powrotu (co ponownie przemawia przeciwko rodzajom drobnych funkcji, które wydają się tutaj zalecane), a to może popierać mój projekt decyzje, ale gdybym zobaczył tę oryginalną funkcję w przeglądzie kodu, w odpowiedzi uzyskałby płomień usenetu w starym stylu.

Smaku jest projektowanie jest trudne do nauczenia i tylko naprawdę pochodzi z doświadczeniem, nie jestem pewien, czy można go sprowadzić do reguł dotyczących długości funkcji, a nawet złożoność cykliczna ma swoje granice jako miarę (czasami rzeczy są po prostu skomplikowane, jakkolwiek sobie z nimi radzisz).
Nie oznacza to, że czysty kod nie omawia pewnych dobrych rzeczy, tak, i należy o tym pomyśleć, ale lokalny zwyczaj i to, co robi istniejąca baza kodu, należy również nadać wagę.

Wydaje mi się, że ten konkretny przykład dotyczy drobiazgowych szczegółów, bardziej martwiłyby mnie rzeczy na wyższym poziomie, ponieważ znacznie bardziej liczy się umiejętność łatwego zrozumienia i debugowania systemu.

Nie umieszczaj wszystkiego w jednej dużej pętli, ale nie rób tego zbyt często:

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Problem z dużą pętlą polega na tym, że naprawdę trudno jest zobaczyć jej ogólną strukturę, gdy obejmuje wiele ekranów. Spróbuj więc wyjąć duże kawałki, najlepiej te, które ponoszą jedną odpowiedzialność i nadają się do ponownego użycia.

Problem z powyższą niewielką funkcją polega na tym, że chociaż atomowość i modułowość są na ogół dobre, to można je posunąć za daleko. Jeśli nie zamierzasz ponownie użyć powyższej funkcji, pogarsza to czytelność kodu i łatwość konserwacji. Aby przejść do szczegółów, musisz przeskoczyć do funkcji, zamiast być w stanie odczytać szczegóły w linii, a wywołanie funkcji zajmuje niewiele mniej miejsca niż sam szczegół.

Oczywiście istnieje równowaga między metodami, które robią za dużo, a metodami, które robią za mało . Nigdy nie wyłamałbym małej funkcji jak wyżej, chyba że miałaby być wywołana z więcej niż jednego miejsca, a nawet wtedy pomyślałbym dwa razy o tym, ponieważ funkcja ta nie jest tak istotna pod względem wprowadzenia nowej logiki takie ledwo gwarantują istnienie.

Wygląda na to, że tak naprawdę chcesz:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER

To powinno być oczywiste dla każdego, kto je głosi: ustawiony phoneNumberdo resourceIdjeżeli jest ona dostępna, lub domyślnie DEV_PHONE_NUMBER, jeśli tak nie jest.

Jeśli naprawdę chcesz ustawić tę zmienną tylko w środowisku produkcyjnym, powinieneś mieć inną, bardziej kanoniczną metodę użyteczności dla aplikacji (która nie wymaga parametrów), aby określić, skąd biegniesz. Czytanie nagłówków tych informacji nie ma sensu.

Pozwólcie mi być szczerzy: wydaje mi się, że wasze środowisko (język / framework / projektowanie klas itp.) Tak naprawdę nie nadaje się do „czystego” kodu. Miksujesz wszystkie możliwe rzeczy w kilku wierszach kodu, które tak naprawdę nie powinny być blisko siebie. Jaką działalność ma jedna funkcja, wiedząc, że resourceId==undefoznacza to, że nie jesteś w produkcji, że używasz domyślnego numeru telefonu w systemach nieprodukcyjnych, że identyfikator zasobu jest zapisany w niektórych „nagłówkach” i tak dalej? Zakładam, że headerssą nagłówkami HTTP, więc decyzję o tym, w jakim środowisku jesteś, pozostawiasz użytkownikowi końcowemu?

Rozkładanie pojedynczych elementów na funkcje nie pomoże ci w rozwiązaniu tego problemu.

Niektóre słowa kluczowe do wyszukania:

• oddzielenie
• spójność
• zastrzyk zależności

Możesz osiągnąć to, czego chcesz (w innych kontekstach) dzięki zerowym wierszom kodu, przesuwając obowiązki kodu i używając nowoczesnych ram (które mogą, ale nie muszą istnieć w twoim środowisku / języku programowania).

Z twojego opisu („300 linii kodu w funkcji głównej”), nawet słowo „funkcja” (zamiast metody) sprawia, że ​​zakładam, że nie ma sensu, co próbujesz osiągnąć. W tym oldskulowym środowisku programistycznym (tj. Podstawowym programowaniu imperatywnym z małą strukturą, z pewnością bez znaczących klas, bez wzorca ram klasowych, takiego jak MVC lub coś takiego), naprawdę nie ma sensu nic robić . Nigdy nie wyjdziesz z miski olejowej bez fundamentalnych zmian. Przynajmniej twój szef pozwala ci tworzyć funkcje pozwalające uniknąć duplikacji kodu, to dobry pierwszy krok!

Znam zarówno rodzaj kodu, jak i typ programisty, który opisujesz całkiem dobrze. Szczerze mówiąc, gdyby był współpracownikiem, moja rada byłaby inna. Ale ponieważ jest to twój szef, nie ma sensu walczyć o to. Nie tylko twój szef może cię unieważnić, ale twoje dodatki do kodu rzeczywiście doprowadzą do gorszego kodu, jeśli tylko częściowo zrobisz „swoje”, a twój szef (i prawdopodobnie inni ludzie) będą postępować tak jak wcześniej. Może być po prostu lepszy wynik końcowy, jeśli dostosujesz się do ich stylu programowania (oczywiście tylko podczas pracy nad tym konkretnym zestawem kodów) i spróbujesz w tym kontekście jak najlepiej go wykorzystać.

„Czysty” jest jednym z celów pisania kodu. To nie jedyny cel. Kolejnym godnym celem jest kolokalność . Mówiąc nieformalnie, kolokalność oznacza, że ​​ludzie próbujący zrozumieć twój kod nie muszą przeskakiwać wszędzie, aby zobaczyć, co robisz. Używanie dobrze nazwanej funkcji zamiast wyrażenia potrójnego może wydawać się dobrą rzeczą, ale w zależności od liczby takich funkcji i ich lokalizacji, ta praktyka może stać się uciążliwa. Nie mogę powiedzieć, czy przekroczyłeś tę granicę, z wyjątkiem stwierdzenia, że ​​jeśli ludzie narzekają, powinieneś słuchać, szczególnie jeśli ci ludzie mają wpływ na twój status zatrudnienia.

Korzystanie z małych funkcji jest ogólnie dobrą praktyką. Ale idealnie uważam, że wprowadzenie funkcji powinno albo oddzielić duże logiczne porcje, albo zmniejszyć ogólny rozmiar kodu poprzez WYSYŁANIE. Podany przez ciebie przykład przedłuża kod i wymaga więcej czasu na przeczytanie przez programistę, podczas gdy krótka alternatywa nie wyjaśnia, że "resourceId"wartość jest obecna tylko w środowisku produkcyjnym. Coś tak prostego jest zarówno łatwe do zapomnienia, jak i kłopotliwe, gdy próbujesz z nim pracować, szczególnie jeśli wciąż jesteś nowy w bazie kodu.

Nie powiem, że powinieneś bezwzględnie używać trójki, niektórzy ludzie, z którymi pracowałem, wolą nieco dłużej if () {...} else {...}, to w większości osobisty wybór. Wolę „podejście jedna linia”, ale zasadniczo trzymam się tego, czego zwykle używa baza kodów.

Jeśli używasz trójskładnika, jeśli kontrola logiczna powoduje, że linia jest zbyt długa lub skomplikowana, rozważ utworzenie dobrze nazwanej zmiennej / s do przechowywania wartości.

// NOTE "resourceId" not present in dev build, use test data
let isProduction = 'resourceId' in headers;
let phoneNumber = isProduction ? headers.resourceId : DEV_PHONE_NUMBER;

Chciałbym również powiedzieć, że jeśli podstawa kodu rozciąga się na 300 funkcji liniowych, to potrzebuje pewnego podziału. Ale radzę używać nieco szerszych pociągnięć.

Podany przykład kodu, twój szef JEST PRAWIDŁOWY. W takim przypadku lepsza jest pojedyncza wyraźna linia.

Ogólnie rzecz biorąc, dzielenie złożonej logiki na mniejsze części jest lepsze dla czytelności, utrzymania kodu i możliwości, że podklasy będą miały inne zachowanie (nawet jeśli tylko nieznacznie).

Nie ignoruj ​​wad: narzut funkcji, zaciemnienie (funkcja nie robi tego, co sugerują komentarze i nazwa funkcji), złożona logika spaghetti, potencjał martwych funkcji (kiedyś zostały utworzone w celu, który już nie jest wywoływany).

Potrafię wymyślić co najmniej dwa argumenty przemawiające za długimi funkcjami:

• Oznacza to, że masz dużo kontekstu wokół każdej linii. Sposób sformalizowania tego: narysuj wykres przepływu kontroli swojego kodu. Na wierzchołku (~ = linia) między wejściem funkcji a wyjściem funkcji znasz wszystkie przychodzące krawędzie. Im dłuższa funkcja, tym więcej takich wierzchołków jest.

• Wiele małych funkcji oznacza, że ​​istnieje większy i bardziej złożony wykres połączeń. Wybierz losową linię w funkcji losowej i odpowiedz na pytanie „w jakim kontekście jest wykonywana ta linia?” Jest to tym trudniejsze, im większy i bardziej złożony jest wykres połączeń, ponieważ musisz spojrzeć na więcej wierzchołków na tym wykresie.

Istnieją również argumenty przeciwko długim funkcjom - przychodzi na myśl testowanie jednostkowe. Skorzystaj z własnego doświadczenia, wybierając między jednym a drugim.

Uwaga: Nie mówię, że twój szef ma rację, tylko że jego perspektywa nie może być całkowicie pozbawiona wartości.

Myślę, że moim zdaniem dobrym parametrem optymalizacji nie jest długość funkcji. Uważam, że desiderata bardziej przydatna do myślenia jest następująca: jeśli wszystko inne jest równe, lepiej jest móc odczytać z kodu ogólny opis zarówno logiki biznesowej, jak i implementacji. (Szczegóły implementacji niskiego poziomu zawsze można odczytać, jeśli znajdziesz odpowiedni fragment kodu).

Komentując odpowiedź Davida Arno :

Pisanie małych funkcji jest uciążliwe, ponieważ zmusza cię do przejścia do każdej małej funkcji, aby zobaczyć, co robi kod.

Jeśli funkcja ma dobrą nazwę, tak nie jest. isApplicationInProduction jest oczywiste i nie powinno być konieczne sprawdzanie kodu, aby zobaczyć, co robi. W rzeczywistości jest odwrotnie: sprawdzenie kodu ujawnia mniej intencji niż nazwa funkcji (dlatego szef musi uciekać się do komentarzy).

Nazwa pokazuje , co oznacza wartość zwracana , ale nie mówi nic o skutkach wykonania kodu (= co robi kod ). Nazwy (tylko) przekazują informacje o intencji , kod przekazuje informacje o zachowaniu (z których części intencji można czasem wywnioskować).

Czasami chcesz jednego, a czasem drugiego, więc ta obserwacja nie tworzy jednostronnie uniwersalnej reguły decyzyjnej.

Umieść wszystko w głównej dużej pętli, nawet jeśli główna pętla ma więcej niż 300 linii, jest szybszy do odczytania

Skanowanie może być szybsze, ale aby naprawdę „odczytać” kod, musisz być w stanie skutecznie wykonać go w swojej głowie. Jest to łatwe z małymi funkcjami i jest naprawdę bardzo trudne z metodami o długości 100 linii.

Zgadzam się, że musisz to zrobić w swojej głowie. Jeśli masz 500 linii funkcji w jednej dużej funkcji w porównaniu do wielu małych funkcji, nie jest dla mnie jasne, dlaczego jest to łatwiejsze.

Załóżmy, że jest to skrajny przypadek 500 wierszy kodu o wysokim skutku ubocznym, a chcesz wiedzieć, czy efekt A wystąpi przed efektem B. Numery linii. W przypadku wielu małych funkcji musisz pamiętać, gdzie w drzewie wywołań występują efekty, a jeśli zapomniałeś, musisz spędzić niebanalną ilość czasu na odkryciu struktury tego drzewa.

Przechodząc przez drzewo wywołań funkcji pomocniczych, stajesz również przed wyzwaniem ustalenia, kiedy przeszedłeś z logiki biznesowej do szczegółów implementacji. Twierdzę bez dowodów *, że im prostszy wykres połączeń, tym łatwiej jest dokonać takiego rozróżnienia.

(*) Przynajmniej jestem co do tego szczery ;-)

Po raz kolejny myślę, że oba podejścia mają mocne i słabe strony.

Napisz tylko małe funkcje, jeśli musisz powielić kod

Nie zgadzam się. Jak pokazuje twój przykład kodu, małe, dobrze nazwane funkcje poprawiają czytelność kodu i powinny być używane, gdy [np.] Nie jesteś zainteresowany „jak”, tylko „co” z części funkcjonalności.

Niezależnie od tego, czy interesuje Cię „jak” lub „co” jest funkcją celu, dla którego czytasz kod (np. Uzyskanie ogólnego pomysłu a wyśledzenie błędu). Cel, do którego czytasz kod, nie jest dostępny podczas pisania programu i najprawdopodobniej przeczytasz kod do różnych celów; różne decyzje zostaną zoptymalizowane do różnych celów.

To powiedziawszy, jest to część poglądu szefa, z którą prawdopodobnie najbardziej się nie zgadzam.

Nie pisz funkcji o nazwie komentarza, umieść złożony wiersz kodu (3-4 wiersze) z komentarzem powyżej. W ten sposób możesz bezpośrednio zmodyfikować uszkodzony kod

Naprawdę nie rozumiem uzasadnienia tego, zakładając, że to naprawdę poważne. [...] Komentarze mają fundamentalną wadę: nie są kompilowane / interpretowane, więc nie mogą być testowane jednostkowo. Kod zostaje zmodyfikowany, a komentarz zostaje sam, a ty nie wiesz, co jest poprawne.

Kompilatory porównują tylko nazwy dla równości, nigdy nie dają Ci błędu wprowadzającego w błąd. Ponadto, ponieważ kilka witryn wywołujących może wywoływać daną funkcję według nazwy, czasami zmiana nazwy jest bardziej uciążliwa i podatna na błędy. Komentarze nie mają tego problemu. Jest to jednak nieco spekulacyjne; aby naprawdę to rozwiązać, prawdopodobnie potrzebowalibyśmy danych o tym, czy programiści częściej aktualizują wprowadzające w błąd komentarze w porównaniu do wprowadzających w błąd nazw, a ja tego nie mam.

Moim zdaniem poprawny kod wymaganej funkcjonalności to:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER;

Lub jeśli chcesz podzielić go na funkcję, prawdopodobnie coś takiego:

phoneNumber = getPhoneNumber(headers);

function getPhoneNumber(headers) {
  return headers.resourceId || DEV_PHONE_NUMBER
}

Myślę jednak, że masz bardziej fundamentalny problem z koncepcją „w produkcji”. Problem z twoją funkcją isApplicationInProductionpolega na tym, że wydaje się dziwne, że jest to jedyne miejsce w systemie, w którym liczy się bycie w „produkcji”, i że zawsze możesz polegać na obecności lub braku nagłówka resourceId, aby ci powiedzieć. Powinna istnieć ogólna isApplicationInProductionmetoda lub getEnvironmentmetoda, która bezpośrednio sprawdza środowisko. Kod powinien wyglądać następująco:

function isApplicationInProduction() {
  process.env.NODE_ENV === 'production';
}

Następnie możesz uzyskać numer telefonu za pomocą:

phoneNumber = isApplicationInProduction() ? headers.resourceId : DEV_PHONE_NUMBER;

Tylko komentarz do dwóch punktów kuli

• Pisanie małych funkcji jest uciążliwe, ponieważ zmusza cię do przejścia do każdej małej funkcji, aby zobaczyć, co robi kod.
• Umieść wszystko w głównej dużej pętli, nawet jeśli główna pętla ma więcej niż 300 linii, jest szybszy do odczytania.

Wiele edytorów (np. IntelliJ) pozwoli ci przejść do funkcji / klasy, klikając Ctrl-Kliknięcie użycia. Ponadto często nie trzeba znać szczegółów implementacji funkcji, aby odczytać kod, dzięki czemu odczyt kodu jest szybszy.

Polecam ci powiedzieć swojemu szefowi; polubi twoje poparcie i zobaczy je jako przywództwo. Po prostu bądź uprzejmy.