Czysty kod: konsekwencje krótkich metod z kilkoma parametrami

Niedawno podczas recenzji kodu natknąłem się na kod napisany przez nowego kolegę, który zawiera wzór z zapachem. Podejrzewam, że decyzje mojego kolegi opierają się na zasadach zaproponowanych w słynnej książce Clean Code (i być może także w innych podobnych książkach).

Rozumiem, że konstruktor klasy jest całkowicie odpowiedzialny za utworzenie ważnego obiektu i że jego głównym zadaniem jest przypisanie właściwości obiektu (prywatnego). Oczywiście może się zdarzyć, że opcjonalne wartości właściwości mogą być ustawione metodami innymi niż konstruktor klasy, ale takie sytuacje są raczej rzadkie (choć niekoniecznie złe, pod warunkiem, że reszta klasy bierze pod uwagę opcjonalność takiej właściwości). Jest to ważne, ponieważ pozwala upewnić się, że obiekt jest zawsze w poprawnym stanie.

Jednak w kodzie, który napotkałem, większość wartości właściwości jest faktycznie ustawiana innymi metodami niż konstruktor. Wartości wynikające z obliczeń są przypisywane właściwościom, które mają być używane w kilku prywatnych metodach w całej klasie. Autor najwyraźniej używa właściwości klasy tak, jakby były zmiennymi globalnymi, które powinny być dostępne w całej klasie, zamiast parametryzować te wartości do funkcji, które ich potrzebują. Dodatkowo metody klasy powinny być wywoływane w określonej kolejności, ponieważ inaczej klasa niewiele by zrobiła.

Podejrzewam, że ten kod został zainspirowany radą, aby metody były krótkie (<= 5 wierszy kodu), aby uniknąć dużych list parametrów (<3 parametrów) i że konstruktory nie mogą działać (np. Wykonywać jakieś obliczenia jest to niezbędne dla ważności obiektu).

Teraz oczywiście mogę uzasadnić ten wzorzec, jeśli mogę udowodnić, że wszelkiego rodzaju nieokreślone błędy potencjalnie powstają, gdy metody nie są wywoływane w określonej kolejności. Przewiduję jednak, że odpowiedzią na to będzie dodanie walidacji, które sprawdzą, czy właściwości muszą zostać ustawione po wywołaniu metod, które wymagają ustawienia tych właściwości.

Wolałbym jednak całkowicie zmienić kod, aby klasa stała się niebieskim drukiem na rzeczywistym obiekcie, a nie serią metod, które należy wywoływać (proceduralnie) w określonej kolejności.

Czuję, że kod, który napotkałem, pachnie. W rzeczywistości uważam, że istnieje dość wyraźne rozróżnienie, kiedy zapisać wartość we właściwości klasy, a kiedy umieścić ją w parametrze dla innej metody do użycia - nie sądzę, że mogą one być alternatywami dla siebie . Szukam słów na to rozróżnienie.

Jako ktoś, kto przeczytał Czysty kod i wiele razy oglądał serię Czyste kodery, i często uczy i trenuje innych ludzi w pisaniu czystszego kodu, naprawdę mogę ręczyć, że twoje obserwacje są prawidłowe - wszystkie wskazane przez ciebie wskaźniki są wymienione w książce .

Jednak w książce poruszono również inne kwestie, które należy również zastosować zgodnie ze wskazanymi przez ciebie wytycznymi. Pozornie zostały one zignorowane w kodzie, z którym mamy do czynienia. Może się tak zdarzyć, ponieważ Twój kolega wciąż znajduje się w fazie uczenia się. W takim przypadku, o ile konieczne jest wskazanie zapachów kodu, warto pamiętać, że robią to z dobrą wolą, uczą się i próbują pisać lepszy kod.

Czysty kod proponuje, aby metody były krótkie, z jak najmniejszą liczbą argumentów. Ale zgodnie z tymi wytycznymi sugeruje, że musimy przestrzegać zasad S OLID, zwiększyć spójność i zmniejszyć sprzężenie .

S w drzewostanach STAŁYCH do pojedynczego Odpowiedzialności zasadę, która stwierdza, że obiekt powinien być odpowiedzialny za tylko jedno. „Rzecz” nie jest bardzo precyzyjnym terminem, więc opisy tej zasady są bardzo różne. Jednak wujek Bob, autor Clean Code, jest także osobą, która wymyśliła tę zasadę, opisując ją jako: „Zbierz razem rzeczy, które zmieniają się z tych samych powodów. Oddziel te rzeczy, które zmieniają się z różnych powodów”. Mówi dalej, co ma na myśli, uzasadniając zmiany tu i tutaj(dłuższe wyjaśnienie byłoby za dużo). Jeśli ta zasada zostanie zastosowana do klasy, z którą mamy do czynienia, bardzo prawdopodobne jest, że elementy zajmujące się obliczeniami zostaną oddzielone od tych, które dotyczą stanu wstrzymania, poprzez podzielenie klasy na dwie lub więcej, w zależności od liczby przyczyn zmienić te obliczenia.

Ponadto klasy Czyste powinny być spójne , co oznacza, że ​​większość jego metod wykorzystuje większość swoich atrybutów. Jako taka, maksymalnie spójna klasa to taka, w której wszystkie metody wykorzystują wszystkie swoje atrybuty; na przykład w aplikacji graficznej możesz mieć Vectorklasę z atrybutami Point ai Point b, gdzie są jedyne metody scaleBy(double factor)i printTo(Canvas canvas)obie działające na obu atrybutach. Natomiast minimalnie spójna klasa to klasa, w której każdy atrybut jest używany tylko w jednej metodzie i nigdy nie więcej niż jeden atrybut jest używany przez każdą metodę. Przeciętnie klasa prezentuje niespójne „grupy” spójnych części - tj. Kilka metod używa atrybutów a, ba cpodczas gdy pozostałe używają cid - co oznacza, że ​​jeśli podzielimy klasę na dwie części, otrzymamy dwa spójne obiekty.

Wreszcie, klasy Clean powinny maksymalnie ograniczyć sprzężenie . Chociaż istnieje wiele rodzajów sprzężeń, o których warto tutaj dyskutować, wydaje się, że w dostępnym kodzie występuje głównie sprzężenie czasowe , w którym, jak wskazałeś, metody obiektu będą działać zgodnie z oczekiwaniami tylko wtedy, gdy zostaną wywołane we właściwej kolejności. I podobnie jak dwie wyżej wspomniane wytyczne, rozwiązania tego zwykle obejmują podział klasy na dwa lub więcej spójnych obiektów. Strategia podziału w tym przypadku zwykle obejmuje wzorce takie jak Builder lub Factory, aw bardzo złożonych przypadkach State-Machines.

TL; DR: Wytyczne dotyczące czystego kodu, których przestrzegał twój kolega, są dobre, ale tylko pod warunkiem przestrzegania pozostałych zasad, praktyk i wzorców wymienionych w książce. Clean wersja „klasy” widzisz będzie podzielony na wiele klas, każda z jednym odpowiedzialności, metody spójnej i bez sprzęgła skroniowej. W tym kontekście sensowne są małe metody i mało argumentów.

Rozumiem, że konstruktor klasy jest całkowicie odpowiedzialny za utworzenie ważnego obiektu i że jego głównym zadaniem jest przypisanie właściwości obiektu (prywatnych).

Zwykle odpowiada za wprowadzenie obiektu w początkowy prawidłowy stan, tak; inne właściwości lub metody mogą następnie zmienić stan na inny prawidłowy.

Jednak w kodzie, który napotkałem, większość wartości właściwości jest faktycznie ustawiana innymi metodami niż konstruktor. Wartości wynikające z obliczeń są przypisywane właściwościom, które mają być używane w kilku prywatnych metodach w całej klasie. Autor najwyraźniej używa właściwości klasy tak, jakby były zmiennymi globalnymi, które powinny być dostępne w całej klasie, zamiast parametryzować te wartości do funkcji, które ich potrzebują. Dodatkowo metody klasy powinny być wywoływane w określonej kolejności, ponieważ inaczej klasa niewiele by zrobiła.

Oprócz problemów z czytelnością i utrzymywalnością, o których się wspominasz, wygląda na to, że w samej klasie odbywa się wiele etapów przepływu / transformacji danych, co może wskazywać, że klasa narusza zasadę pojedynczej odpowiedzialności.

podejrzewam, że ten kod został zainspirowany radą, aby metody były krótkie (<= 5 linii kodu), aby uniknąć dużych list parametrów (<3 parametrów) i że konstruktorzy nie mogą działać (np. wykonywać obliczenia, które ma zasadnicze znaczenie dla ważności obiektu).

Postępowanie zgodnie z niektórymi wytycznymi dotyczącymi kodowania, a ignorowanie innych często prowadzi do głupiego kodu. Jeśli chcemy na przykład uniknąć wykonywania pracy przez konstruktora, rozsądnym sposobem byłoby zazwyczaj wykonanie pracy przed budową i przekazanie wyniku tej pracy konstruktorowi. (Jednym z argumentów za tym podejściem może być to, że unikasz powierzania swojej klasie dwóch obowiązków: pracy nad jej inicjalizacją i „głównej pracy”, cokolwiek to jest.)

Z mojego doświadczenia wynika, że ​​tworzenie małych klas i metod rzadko jest czymś, co muszę brać pod uwagę jako odrębną kwestię - raczej wynika to w sposób naturalny z pojedynczej odpowiedzialności.

Wolałbym jednak całkowicie zmienić kod, aby klasa stała się niebieskim drukiem na rzeczywistym obiekcie, a nie serią metod, które należy wywoływać (proceduralnie) w określonej kolejności.

Prawdopodobnie miałbyś rację. Nie ma nic złego w pisaniu prostego kodu proceduralnego; istnieje problem z nadużywaniem paradygmatu OO do pisania zaciemnionego kodu proceduralnego.

Wierzę, że istnieje dość wyraźne rozróżnienie, kiedy zapisać wartość we właściwości klasy, a kiedy umieścić ją w parametrze dla innej metody do użycia - nie sądzę, że mogą one być dla siebie alternatywami. Szukam słów na to rozróżnienie.

Zwykle nie powinieneś umieszczać wartości w polu jako sposobu przekazywania jej z jednej metody do drugiej; wartość w polu powinna być znaczącą częścią stanu obiektu w danym czasie. (Mogę wymyślić kilka ważnych wyjątków, ale nie takich, w których takie metody są publiczne lub gdzie istnieje zależność od kolejności, o której użytkownik klasy powinien wiedzieć)

Najprawdopodobniej poręczycie się tutaj w złym wzorze. Małe funkcje i niski poziom arsenału same w sobie rzadko stanowią problem. Prawdziwym problemem jest tutaj sprzężenie, które powoduje zależność od kolejności między funkcjami, więc poszukaj sposobów rozwiązania tego problemu, nie tracąc przy tym zalet małych funkcji.

Kod mówi głośniej niż słowa. Ludzie otrzymują tego rodzaju poprawki znacznie lepiej, jeśli faktycznie można wykonać część refaktoryzacji i pokazać poprawę, być może jako ćwiczenie programowania w parach. Kiedy to robię, często uważam, że trudniej jest pomyśleć o poprawnym zaprojektowaniu, równoważeniu wszystkich kryteriów.