Czysty kod i obiekty hybrydowe oraz zazdrość o cechy

10

Niedawno dokonałem poważnych zmian w moim kodzie. Jedną z głównych rzeczy, które próbowałem zrobić, było podzielenie moich klas na obiekty danych i obiekty robocze. Zostało to zainspirowane między innymi przez tę sekcję Clean Code :

Hybrydy

To zamieszanie czasami prowadzi do niefortunnych hybrydowych struktur danych, które są w połowie obiektowymi i w połowie strukturami danych. Mają funkcje, które robią znaczące rzeczy, a także mają zmienne publiczne lub publiczne akcesory i mutatory, które pod każdym względem upubliczniają zmienne prywatne, kusząc inne funkcje zewnętrzne do używania tych zmiennych w sposób, w jaki program proceduralny użyłby struktura danych.

Takie hybrydy utrudniają dodawanie nowych funkcji, ale także utrudniają dodawanie nowych struktur danych. Są najgorsze z obu światów. Unikaj ich tworzenia. Wskazują na zagmatwany projekt, którego autorzy nie są pewni - lub gorzej, nie wiedzą - czy potrzebują ochrony przed funkcjami lub typami.

Ostatnio patrzyłem na kod jednego z moich obiektów roboczych (który zdarza się zaimplementować wzorzec gościa ) i zobaczyłem to:

@Override
public void visit(MarketTrade trade) {
    this.data.handleTrade(trade);
    updateRun(trade);
}

private void updateRun(MarketTrade newTrade) {
    if(this.data.getLastAggressor() != newTrade.getAggressor()) {
        this.data.setRunLength(0);
        this.data.setLastAggressor(newTrade.getAggressor());
    }
    this.data.setRunLength(this.data.getRunLength() + newTrade.getLots());
}

Natychmiast powiedziałem sobie: „zazdrość o cechy! Ta logika powinna być w Dataklasie - szczególnie w handleTrademetodzie. handleTradeI zawszeupdateRun powinna się zdarzyć razem”. Ale potem pomyślałem: „klasa danych jest tylko strukturą danych, jeśli zacznę to robić, to stanie się Obiektem Hybrydowym!”public

Co jest lepsze i dlaczego? Jak decydujesz, co zrobić?

durron597
źródło
2
Dlaczego „dane” w ogóle muszą być strukturą danych. Ma jasne zachowanie. Wyłącz więc wszystkie pobieracze i ustawiacze, aby żaden obiekt nie mógł manipulować stanem wewnętrznym.
Cormac Mulhall

Odpowiedzi:

9

Cytowany tekst ma dobrą radę, chociaż „struktury danych” zastąpię „rekordami”, zakładając, że chodzi o coś takiego jak struktury. Rekordy to tylko głupie agregacje danych. Chociaż mogą być zmienne (a więc stanowe w sposobie myślenia funkcjonalnego programowania), nie mają żadnego wewnętrznego stanu, żadnych niezmienników, które należy chronić. Całkowicie poprawne jest dodawanie operacji do rekordu, które ułatwiają jego użycie.

Na przykład możemy argumentować, że wektor 3D jest głupim zapisem. Nie powinno to jednak powstrzymywać nas przed dodaniem metody podobnej add, co ułatwia dodawanie wektorów. Dodanie zachowania nie zamienia (nie tak głupiego) rekordu w hybrydę.

Ta linia zostaje przekroczona, gdy publiczny interfejs obiektu pozwala nam przerwać enkapsulację: Istnieją pewne elementy wewnętrzne, do których możemy uzyskać bezpośredni dostęp, powodując w ten sposób niepoprawny stan obiektu. Wydaje mi się, że Datama stan i że można go doprowadzić do stanu nieprawidłowego:

  • Po zakończeniu transakcji ostatni agresor może nie zostać zaktualizowany.
  • Ostatniego agresora można zaktualizować, nawet jeśli nie nastąpił nowy handel.
  • Długość przebiegu może zachować starą wartość, nawet gdy agresor został zaktualizowany.
  • itp.

Jeśli jakikolwiek stan jest poprawny dla twoich danych, wtedy wszystko jest w porządku z twoim kodem i możesz kontynuować. W przeciwnym razie: DataKlasa odpowiada za swoją spójność danych. Jeśli obsługa handlu zawsze wiąże się z aktualizacją agresora, takie zachowanie musi należeć do Dataklasy. Jeśli zmiana agresora wiąże się z ustawieniem długości przebiegu na zero, takie zachowanie musi być częścią Dataklasy. Datanigdy nie był głupi. Już uczyniłeś go hybrydą, dodając podmioty ustawiające.

Jest jeden scenariusz, w którym możesz rozważyć złagodzenie tych ścisłych obowiązków: Jeśli Datajest prywatny dla twojego projektu i dlatego nie jest częścią żadnego publicznego interfejsu, nadal możesz zapewnić prawidłowe użycie klasy. To jednak nakłada odpowiedzialność za zachowanie Dataspójności w całym kodzie, zamiast gromadzenia ich w centralnej lokalizacji.

Niedawno napisałem odpowiedź na temat enkapsulacji , która dokładniej opisuje, czym jest enkapsulacja i jak można to zapewnić.

amon
źródło
5

Fakt, że handleTrade()i updateRun()zawsze dzieje się razem (a druga metoda jest w rzeczywistości na odwiedzającym i wywołuje kilka innych metod na obiekcie danych) pachnie sprzężeniem czasowym . Oznacza to, że musisz wywoływać metody w określonej kolejności, i zgaduję, że wywołanie metod poza kolejnością zniszczy coś w najgorszym przypadku lub nie zapewni znaczącego wyniku w najlepszym przypadku. Niedobrze.

Zazwyczaj poprawnym sposobem na przeformułowanie tej zależności jest, aby każda metoda zwróciła wynik, który można albo zastosować do następnej metody, albo zastosować bezpośrednio.

Stary kod:

MyObject x = ...;
x.actionOne();
x.actionTwo();
String result = x.actionThree();

Nowy kod:

MyObject x = ...;
OneResult r1 = x.actionOne();
TwoResult r2 = r1.actionTwo();
String result = r2.actionThree();

Ma to kilka zalet:

  • Przenosi oddzielne obawy do oddzielnych obiektów ( SRP ).
  • Usuwa sprzężenie czasowe: niemożliwe jest wywoływanie metod poza kolejnością, a podpisy metod zapewniają niejawną dokumentację dotyczącą ich wywoływania. Czy kiedykolwiek przeglądałeś dokumentację, widziałeś pożądany obiekt i pracowałeś wstecz? Chcę obiekt Z. Ale potrzebuję litery Y, aby uzyskać literę Z. Aby uzyskać literę Y, potrzebuję litery X. Aha! Mam W, która jest wymagana, aby uzyskać X. Połącz to wszystko razem, a twoje W może być teraz użyte do uzyskania Z.
  • Dzielenie takich obiektów z większym prawdopodobieństwem czyni je niezmiennymi, co ma wiele zalet wykraczających poza zakres tego pytania. Szybkie na wynos jest to, że niezmienne obiekty zwykle prowadzą do bezpieczniejszego kodu.

źródło
Nie ma czasowego sprzężenia między tymi dwoma wywołaniami metod. Zamień ich kolejność, a zachowanie się nie zmieni.
durron597,
1
Początkowo myślałem o sprzężeniu sekwencyjnym / czasowym podczas czytania pytania, ale zauważyłem, że updateRunmetoda była prywatna . Unikanie łączenia sekwencyjnego jest dobrą radą, ale dotyczy tylko projektowania API / publicznych interfejsów, a nie szczegółów implementacji. Rzeczywistym pytaniem wydaje się być to, czy updateRunpowinien on być gościem czy klasą danych, i nie rozumiem, w jaki sposób ta odpowiedź rozwiązuje ten problem.
amon
Widoczność updateRunjest nieistotna, ważne jest wdrożenie, this.dataktórego nie ma w pytaniu i obiekt jest manipulowany przez obiekt gościa.
Jeśli już, fakt, że ten gość po prostu dzwoni do grupy osób ustawiających i nie przetwarza niczego, jest przyczyną braku połączenia czasowego. Prawdopodobnie nie ma znaczenia, w jakiej kolejności są wywoływani setery.
0

Z mojego punktu widzenia klasa powinna zawierać „wartości dla stanu (zmienne składowe) i implementacje zachowania (funkcje składowe, metody)”.

„Niefortunne hybrydowe struktury danych” pojawiają się, jeśli upublicznisz zmienne składowe stanu klasy (lub ich metody pobierające / ustawiające), które nie powinny być publiczne.

Nie widzę więc potrzeby posiadania oddzielnych klas dla obiektów danych i obiektów roboczych.

Powinieneś być w stanie zachować niepubliczne zmienne będące elementami stanu (twoja warstwa bazy danych powinna być w stanie obsługiwać niepubliczne zmienne będące elementami)

Zazdrość o cechy to klasa, która nadmiernie używa metod innej klasy. Zobacz Code_smell . Posiadanie klasy z metodami i stanem wyeliminowałoby to.

k3b
źródło