Czy metody prywatne z jednym odniesieniem są w złym stylu?

139

Zasadniczo używam prywatnych metod do enkapsulacji funkcjonalności, która jest ponownie wykorzystywana w wielu miejscach w klasie. Ale czasami mam dużą metodę publiczną, którą można by podzielić na mniejsze etapy, każdy według własnej metody prywatnej. Spowoduje to, że metoda publiczna będzie krótsza, ale obawiam się, że zmuszanie każdego, kto czyta metodę, do przeskakiwania do różnych metod prywatnych, pogorszy czytelność.

Czy istnieje konsensus w tej sprawie? Czy lepiej jest mieć długie metody publiczne, czy też podzielić je na mniejsze części, nawet jeśli nie można ponownie użyć każdego z nich?

Jordak
źródło
7
Wszystkie odpowiedzi są oparte na opiniach. Oryginalni autorzy zawsze myślą, że ich kod ravioli jest bardziej czytelny; kolejni redaktorzy nazywają to ravioli.
Frank Hileman,
5
Proponuję przeczytać Czysty kod. Poleca ten styl i ma wiele przykładów. Ma również rozwiązanie problemu „przeskakiwania”.
Kat
1
Myślę, że to zależy od twojego zespołu i zawartych w nim linii kodu.
Mam nadzieję, że będzie
1
Czy cała struktura STRUTS oparta jest na jednorazowych metodach Gettera / Settera, z których wszystkie są metodami jednorazowego użytku?
Zibbobz

Odpowiedzi:

203

Nie, to nie jest zły styl. W rzeczywistości jest to bardzo dobry styl.

Funkcje prywatne nie muszą istnieć tylko ze względu na możliwość ponownego użycia. To z pewnością jeden dobry powód do ich stworzenia, ale jest jeszcze jeden: rozkład.

Rozważ funkcję, która robi za dużo. Ma on sto linii długości i nie sposób o nim myśleć.

Jeśli podzielisz tę funkcję na mniejsze części, nadal „będzie” tyle samo, co wcześniej, ale na mniejsze części. Wywołuje inne funkcje, które powinny mieć opisowe nazwy. Główna funkcja brzmi prawie jak książka: wykonaj A, następnie B, a następnie C itd. Funkcje, które wywołuje, mogą być wywoływane tylko w jednym miejscu, ale teraz są mniejsze. Każda konkretna funkcja jest koniecznie wyodrębniona z innych funkcji: mają różne zakresy.

Kiedy rozkładasz duży problem na mniejsze problemy, nawet jeśli te mniejsze problemy (funkcje) są używane / rozwiązane tylko raz, zyskujesz kilka korzyści:

  • Czytelność. Nikt nie może odczytać funkcji monolitycznej i zrozumieć, co ona robi całkowicie. Możesz dalej okłamywać siebie lub podzielić je na kawałki wielkości kęsa, które mają sens.

  • Lokalizacja odniesienia. Teraz niemożliwe staje się zadeklarowanie i użycie zmiennej, a następnie pozostawienie jej dookoła i ponowne użycie 100 linii później. Funkcje te mają różne zakresy.

  • Testowanie. Chociaż konieczne jest jedynie testowanie jednostkowe członków publicznych klasy, pożądane może być również przetestowanie niektórych członków prywatnych. Jeśli istnieje krytyczna sekcja długiej funkcji, która może skorzystać z testowania, niemożliwe jest przetestowanie jej niezależnie bez wyodrębnienia jej do osobnej funkcji.

  • Modułowość. Teraz, gdy masz funkcje prywatne, możesz znaleźć jedną lub więcej z nich, które można wyodrębnić do osobnej klasy, bez względu na to, czy jest używana tylko tutaj, czy może być ponownie użyta. Do poprzedniego punktu ta oddzielna klasa będzie prawdopodobnie łatwiejsza do przetestowania, ponieważ będzie potrzebować publicznego interfejsu.

Pomysł podzielenia dużego kodu na mniejsze, łatwiejsze do zrozumienia i przetestowania, jest kluczowym punktem książki wuja Boba „Czysty kod” . W chwili pisania tej odpowiedzi książka ma dziewięć lat, ale jest tak samo aktualna jak wtedy.


źródło
7
Nie zapomnij o utrzymaniu spójnego poziomu abstrakcji i dobrych nazwisk, które zapewnią, że wgląd w metody nie zaskoczy Cię. Nie zdziw się, jeśli ukryje się w nim cały obiekt.
candied_orange
14
Czasami metody dzielenia mogą być dobre, ale utrzymywanie rzeczy w szeregu może również mieć zalety. Jeśli na przykład kontrola graniczna mogłaby sensownie zostać wykonana wewnątrz lub na zewnątrz bloku kodu, utrzymanie tego bloku kodu w linii ułatwi sprawdzenie, czy kontrola granic jest wykonywana raz, więcej niż raz, czy wcale . Przeciągnięcie bloku kodu do własnego podprogramu utrudniłoby ustalenie takich rzeczy.
supercat
7
Punkt „czytelność” można nazwać „Abstrakcja”. Chodzi o abstrakcję. „Kawałki wielkości kęsa” robią jedną rzecz , jedną rzecz niskopoziomową, której tak naprawdę nie obchodzą drobiazgowe szczegóły podczas czytania lub przechodzenia przez metodę publiczną. Wyodrębniając ją do funkcji, można przejść nad nią i skupić się na dużym schemacie rzeczy / tego, co robi metoda publiczna na wyższym poziomie.
Mathieu Guindon
7
Kula „Testowanie” jest kwestionowana przez IMO. Jeśli Twoi prywatni członkowie chcą zostać przetestowani, prawdopodobnie należą oni do swojej klasy, jako członkowie publiczni. Oczywiście pierwszym krokiem do wyodrębnienia klasy / interfejsu jest wyodrębnienie metody.
Mathieu Guindon
6
Podział funkcji wyłącznie według numerów linii, bloków kodu i zakresu zmiennej bez dbania o rzeczywistą funkcjonalność to straszny pomysł, i argumentowałbym, że lepszą alternatywą jest pozostawienie jej w jednej funkcji. Powinieneś dzielić funkcje według funkcji. Zobacz odpowiedź DaveGauera . W pewnym sensie sugerujesz to w swojej odpowiedzi (z podaniem nazwisk i problemów), ale nie mogę się powstrzymać, ale czuję, że ten aspekt wymaga dużo większego skupienia, biorąc pod uwagę jego wagę i znaczenie w odniesieniu do pytania.
Dukeling,
36

To chyba świetny pomysł!

Mam problem z dzieleniem długich liniowych sekwencji akcji na osobne funkcje wyłącznie w celu zmniejszenia średniej długości funkcji w bazie kodu:

function step1(){
  // ...
  step2(zarb, foo, biz);
}

function step2(zarb, foo, biz){
  // ...
  step3(zarb, foo, biz, gleep);
}

function step3(zarb, foo, biz, gleep){
  // ...
}

Teraz faktycznie dodałeś wiersze źródła i znacznie zmniejszyłeś całkowitą czytelność. Zwłaszcza jeśli przekazujesz teraz wiele parametrów między każdą funkcją, aby śledzić stan. Yikes!

Jeśli jednak udało Ci się wyodrębnić jedną lub więcej linii do czystej funkcji, która służy jednemu, wyraźnemu celowi ( nawet jeśli wywoływana jest tylko raz ), to poprawiłeś czytelność:

function foo(){
  f = getFrambulation();
  g = deglorbFramb(f);
  r = reglorbulate(g);
}

Prawdopodobnie nie będzie to łatwe w rzeczywistych sytuacjach, ale elementy czystej funkcjonalności można często wypróbować, jeśli się nad tym zastanowić wystarczająco długo.

Będziesz wiedział, że jesteś na dobrej drodze, gdy masz funkcje z ładnymi nazwami czasowników i kiedy ich funkcja rodzica wywołuje je, a cała rzecz praktycznie brzmi jak akapit prozy.

Potem, kiedy wrócisz kilka tygodni później, aby dodać więcej funkcji i okaże się, że możesz ponownie użyć jednej z tych funkcji, to och, zachwycająca radość! Cóż za cudowna promienna radość!

DaveGauer
źródło
2
Słyszałem ten argument od wielu lat i nigdy nie rozgrywa się on w prawdziwym świecie. Mówię konkretnie o jednej lub dwóch funkcjach liniowych, wywoływanych tylko z jednego miejsca. Podczas gdy oryginalny autor zawsze uważa, że ​​jest „bardziej czytelny”, kolejni redaktorzy często nazywają to kodem ravioli. Zasadniczo zależy to od głębokości połączenia.
Frank Hileman,
34
@FrankHileman Szczerze mówiąc, nigdy nie widziałem, aby jakikolwiek programista komplementował kod swojego poprzednika.
T. Sar
4
@TSar poprzedni programista jest źródłem wszystkich problemów!
Frank Hileman,
18
@TSar Nigdy nie pochwaliłem poprzedniego programisty, gdy byłem poprzednim programistą.
IllusiveBrian
3
foo = compose(reglorbulate, deglorbFramb, getFrambulation);
Zaletą
19

Odpowiedź jest bardzo zależna od sytuacji. @Snowman obejmuje pozytywne skutki zerwania dużej funkcji publicznej, ale ważne jest, aby pamiętać, że mogą również mieć negatywne skutki, o czym słusznie się martwisz.

  • Wiele prywatnych funkcji z efektami ubocznymi może sprawić, że kod będzie trudny do odczytania i dość kruchy. Jest to szczególnie prawdziwe, gdy te prywatne funkcje zależą od siebie nawzajem. Unikaj ściśle powiązanych funkcji.

  • Abstrakcje są nieszczelne . Chociaż udaje się udawać, jak wykonywana jest operacja lub jak przechowywane są dane, nie ma to znaczenia, ale są przypadki, w których to robi, i ważne jest, aby je zidentyfikować.

  • Semantyka i kontekst mają znaczenie. Jeśli nie uda się wyraźnie uchwycić działania funkcji w jej nazwie, ponownie można zmniejszyć czytelność i zwiększyć kruchość funkcji publicznej. Jest to szczególnie prawdziwe, gdy zaczynasz tworzyć funkcje prywatne z dużą liczbą parametrów wejściowych i wyjściowych. Podczas gdy z funkcji publicznej widzisz, jakie funkcje prywatne wywołuje, z funkcji prywatnej nie widzisz, jakie funkcje publiczne ją nazywają. Może to prowadzić do „naprawy błędów” w funkcji prywatnej, która psuje funkcję publiczną.

  • Mocno rozłożony kod jest zawsze bardziej przejrzysty dla autora niż dla innych. Nie oznacza to, że nie może to być dla innych jasne, ale łatwo powiedzieć, że ma doskonały sens, że bar()należy to sprawdzić foo()w momencie pisania.

  • Niebezpieczne ponowne użycie. Twoja funkcja publiczna prawdopodobnie ograniczyła dostęp do każdej funkcji prywatnej. Jeśli te założenia wejściowe nie zostaną poprawnie przechwycone (dokumentowanie WSZYSTKICH założeń jest trudne), ktoś może niewłaściwie użyć jednej z twoich prywatnych funkcji, wprowadzając błędy w bazie kodu.

Rozkładaj funkcje na podstawie ich wewnętrznej spójności i sprzężenia, a nie długości bezwzględnej.

dlasalle
źródło
6
Zignoruj ​​czytelność, spójrzmy na aspekt zgłaszania błędów: jeśli użytkownik zgłasza, że ​​wystąpił błąd MainMethod(dość łatwy do zdobycia, zwykle zawiera symbole i powinieneś mieć plik dereferencji symboli), który ma 2k linii (numery linii nigdy nie są uwzględniane raporty o błędach) i jest to NRE, które może się zdarzyć przy 1k tych wierszy, no cóż, niech mnie diabli, jeśli przyjrzę się temu. Ale jeśli powiedzą mi, że to się wydarzyło SomeSubMethodAi ma 8 wierszy, a wyjątkiem był NRE, prawdopodobnie znajdę i naprawię to dość łatwo.
410_Gone
2

IMHO, wartość wyciągania bloków kodu wyłącznie w celu rozbicia złożoności, często wiąże się z różnicą złożoności między:

  1. Pełny i dokładny opis w języku ludzkim tego, co robi kod, w tym sposób , w jaki obsługuje przypadki narożne oraz

  2. Sam kod.

Jeśli kod jest znacznie bardziej skomplikowany niż opis, zastąpienie kodu wbudowanego wywołaniem funkcji może ułatwić zrozumienie otaczającego kodu. Z drugiej strony niektóre pojęcia można wyrazić bardziej czytelnie w języku komputerowym niż w języku ludzkim. Uważałbym w=x+y+z;na przykład za bardziej czytelny niż w=addThreeNumbersAssumingSumOfFirstTwoDoesntOverflow(x,y,z);.

W miarę podziału dużej funkcji będzie coraz mniej różnicy między złożonością podfunkcji a ich opisami, a przewaga kolejnych podziałów będzie się zmniejszać. Jeśli rzeczy zostaną podzielone do tego stopnia, że ​​opisy będą bardziej skomplikowane niż kod, dalsze podziały pogorszą kod.

supercat
źródło
2
Twoja nazwa funkcji wprowadza w błąd. Na w = x + y + z nie ma nic, co wskazuje na jakąkolwiek kontrolę przepełnienia, podczas gdy sama nazwa metody wydaje się sugerować inaczej. Lepszą nazwą dla Twojej funkcji byłoby na przykład „addAll (x, y, z)”, lub nawet po prostu „Sum (x, y, z)”.
T. Sar
6
Ponieważ mówimy o metodach prywatnych , pytanie to prawdopodobnie nie odnosi się do C. W każdym razie, nie o to mi chodzi - możesz po prostu nazwać tę samą funkcję „sumą” bez potrzeby zajmowania się założeniem dotyczącym podpisu metody. Zgodnie z logiką każda metoda rekurencyjna powinna na przykład nazywać się „DoThisAssumingStackDoesntOverflow”. To samo dotyczy „FindSubstringAssumingStartingPointIsInsideBounds”.
T. Sar
1
Innymi słowy, podstawowe założenia, cokolwiek by to nie było (dwie liczby się nie przepełnią, stos się nie przepełni, indeks został poprawnie przekazany) nie powinny występować w nazwie metody. Te rzeczy powinny być oczywiście udokumentowane, jeśli to konieczne, ale nie na podpisie metody.
T. Sar
1
@TSar: Byłem trochę żartobliwy z tą nazwą, ale najważniejsze jest to, że (przejście na uśrednianie, ponieważ jest to lepszy przykład) programista, który widzi „(x + y + z + 1) / 3” w kontekście będzie: o wiele lepiej umie wiedzieć, czy jest to odpowiedni sposób obliczenia średniej, biorąc pod uwagę kod wywołujący, niż ktoś, kto patrzy na definicję lub stronę wywoływania int average3(int n1, int n2, int n3). Rozdzielenie funkcji „średniej” oznaczałoby, że ktoś, kto chciałby sprawdzić, czy jest ona odpowiednia do wymagań, musiałby szukać w dwóch miejscach.
supercat
1
Jeśli weźmiemy na przykład C #, będziesz mieć tylko jedną metodę „średniej”, którą można zaakceptować dowolną liczbę parametrów. W rzeczywistości w c # działa na dowolnym zbiorze liczb - i jestem pewien, że jest to o wiele łatwiejsze do zrozumienia niż zajmowanie się surową matematyką w kodzie.
T. Sar
2

Jest to wyzwanie równoważące.

Lepsze jest lepsze

Metoda prywatna skutecznie zapewnia nazwę zawartego kodu, a czasem znaczącą sygnaturę (chyba że połowa jego parametrów to całkowicie dane ad-hoc tramp z niejasnymi, nieudokumentowanymi współzależnościami).

Nadawanie nazw konstrukcjom kodu jest na ogół dobre, pod warunkiem, że nazwy sugerują znaczącą umowę dla osoby wywołującej, a umowa metody prywatnej dokładnie odpowiada temu, co sugeruje nazwa.

Zmuszając się do myślenia o znaczących umowach dotyczących mniejszych części kodu, początkowy programista może wykryć niektóre błędy i uniknąć ich bezboleśnie nawet przed testowaniem przez programistów. Działa to tylko wtedy, gdy programista dąży do zwięzłego nazewnictwa (proste brzmienie, ale dokładne) i jest skłonny dostosować granice metody prywatnej, aby zwięzłe nazewnictwo było nawet możliwe.

Późniejsza konserwacja jest pomogło, także dlatego, że dodatkowy nazewnictwa pomaga uczynić kod siebie dokumentowanie .

  • Kontrakty na małe fragmenty kodu
  • Metody wyższego poziomu czasami zamieniają się w krótką sekwencję wywołań, z których każda robi coś znaczącego i wyraźnie nazwanego - w połączeniu z cienką warstwą ogólnej, najbardziej zewnętrznej obsługi błędów. Taka metoda może stać się cennym zasobem szkoleniowym dla każdego, kto musi szybko stać się ekspertem w zakresie ogólnej struktury modułu.

Dopóki nie będzie zbyt dobrze

Czy można przesadzić nadawanie nazw małym fragmentom kodu i skończyć na zbyt wielu, zbyt małych prywatnych metodach? Pewnie. Co najmniej jeden z następujących symptomów wskazuje, że metody stają się zbyt małe, aby były przydatne:

  • Zbyt wiele przeciążeń, które tak naprawdę nie reprezentują alternatywnych podpisów dla tej samej podstawowej logiki, ale raczej jeden stały stos wywołań.
  • Synonimy używane tylko w celu wielokrotnego odwoływania się do tej samej koncepcji („zabawne nazywanie”)
  • Wiele ozdobnych nazw dekoracyjnych, takich jak XxxInternallub DoXxx, zwłaszcza jeśli nie ma jednolitego schematu ich wprowadzania.
  • Niezdarne nazwy prawie dłuższe niż sama implementacja, jak LogDiskSpaceConsumptionUnlessNoUpdateNeeded
Jirka Hanika
źródło
1

W przeciwieństwie do tego, co powiedzieli inni, twierdzę, że długa metoda publiczna to zapach projektowy, którego nie można naprawić przez rozkład na prywatne metody.

Ale czasami mam dużą publiczną metodę, którą można podzielić na mniejsze etapy

Jeśli tak jest, to argumentowałbym, że każdy krok powinien być własnym obywatelem pierwszej klasy, z jedną odpowiedzialnością. W paradygmacie zorientowanym obiektowo sugerowałbym utworzenie interfejsu i implementacji dla każdego kroku w taki sposób, aby każdy z nich miał jedną, łatwą do zidentyfikowania odpowiedzialność i mógł być nazwany w taki sposób, aby odpowiedzialność była jasna. Umożliwia to testowanie jednostkowe (dawniej) dużej metody publicznej, a także każdego pojedynczego kroku niezależnie od siebie. Wszystkie powinny być również udokumentowane.

Dlaczego nie rozłożyć na prywatne metody? Oto kilka powodów:

  • Ścisłe połączenie i testowalność. Zmniejszając rozmiar metody publicznej poprawiłeś jej czytelność, ale cały kod jest nadal ściśle ze sobą powiązany. Możesz testować jednostkowo poszczególne metody prywatne (przy użyciu zaawansowanych funkcji środowiska testowego), ale nie możesz łatwo przetestować metody publicznej niezależnie od metod prywatnych. Jest to sprzeczne z zasadami testów jednostkowych.
  • Wielkość klasy i złożoność. Zmniejszyłeś złożoność metody, ale zwiększyłeś złożoność klasy. Metoda publiczna jest łatwiejsza do odczytania, ale klasa jest trudniejsza do odczytania, ponieważ ma więcej funkcji, które definiują jej zachowanie. Preferuję małe klasy z pojedynczą odpowiedzialnością, więc długa metoda jest znakiem, że klasa robi za dużo.
  • Nie można łatwo ponownie użyć. Często zdarza się, że jako fragment kodu dojrzewa przydatna jest możliwość ponownego użycia. Jeśli twoje kroki są metodami prywatnymi, nie można ich użyć nigdzie indziej bez wcześniejszego ich wyodrębnienia. Dodatkowo może zachęcać do kopiowania i wklejania, gdy potrzebny jest krok w innym miejscu.
  • Podział w ten sposób może być arbitralny. Twierdziłbym, że podział długiej metody publicznej nie bierze tyle uwagi do przemyślenia czy zaprojektowania, jakbyś miał podzielić zadania na klasy. Każda klasa musi być uzasadniona odpowiednią nazwą, dokumentacją i testami, podczas gdy prywatna metoda nie bierze pod uwagę tak dużej uwagi.
  • Ukrywa problem. Więc postanowiłeś podzielić swoją metodę publiczną na małe metody prywatne. Teraz nie ma problemu! Możesz dodawać coraz więcej kroków, dodając coraz więcej prywatnych metod! Przeciwnie, uważam, że jest to poważny problem. Ustanawia wzorzec zwiększania złożoności klasy, po którym następują kolejne poprawki błędów i implementacje funkcji. Wkrótce twoje metody prywatne będzie rosnąć i one będą musiały być podzielone.

ale martwię się, że zmuszanie każdego, kto czyta tę metodę, do przeskakiwania na różne prywatne metody, pogorszy czytelność

To jest argument, który miałem ostatnio z jednym z moich kolegów. Twierdzi, że posiadanie całego zachowania modułu w tym samym pliku / metodzie poprawia czytelność. Zgadzam się, że kod jest łatwiejszy do naśladowania, gdy wszystko razem, ale kod jest trudniejszy do uzasadnienia w miarę wzrostu złożoności. W miarę rozwoju systemu staje się coraz trudniejsze rozumowanie całego modułu jako całości. Kiedy rozkładasz złożoną logikę na kilka klas, z których każda ma jedną odpowiedzialność, wtedy znacznie łatwiej jest zrozumieć każdą część.

Samuel
źródło
1
Musiałbym się nie zgodzić. Zbyt duża lub przedwczesna abstrakcja prowadzi do niepotrzebnie złożonego kodu. Prywatna metoda może być pierwszym krokiem na ścieżce, która może wymagać połączenia i abstrakcji, ale twoje podejście tutaj sugeruje wędrowanie po miejscach, które nigdy nie muszą być odwiedzane.
WillC
1
Rozumiem, skąd pochodzisz, ale myślę, że kod pachnie tak, jak ten, o który prosi OP, są znakami, że jest gotowy na lepszy projekt. Przedwczesna abstrakcja polegałaby na zaprojektowaniu tych interfejsów, zanim jeszcze napotkasz ten problem.
Samuel
Zgadzam się z tym podejściem. W rzeczywistości, gdy postępujesz zgodnie z TDD, jest to naturalny postęp. Druga osoba twierdzi, że jest to zbyt przedwczesna abstrakcja, ale znacznie łatwiej jest mi kpić z funkcji, której potrzebuję w osobnej klasie (za interfejsem), niż w rzeczywistości zaimplementować ją w metodzie prywatnej, aby test mógł zostać zaliczony .
Eternal21