Ile logiki w Gettersach

Moi współpracownicy mówią mi, że w getterach i seterach powinna być jak najmniej logiki.

Jestem jednak przekonany, że w programach pobierających i ustawiających można ukryć wiele rzeczy, aby chronić użytkowników / programistów przed szczegółami implementacji.

Przykład tego, co robię:

public List<Stuff> getStuff()
{
   if (stuff == null || cacheInvalid())
   {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

Przykład tego, jak praca każe mi robić różne rzeczy (cytują „Czysty kod” od wuja Boba):

public List<Stuff> getStuff()
{
    return stuff;
}

public void loadStuff()
{
    stuff = getStuffFromDatabase();
}

Jaka logika jest odpowiednia w seter / getter? Jaki jest pożytek z pustych programów pobierających i ustawiających oprócz naruszenia ukrywania danych?

Sposób, w jaki praca każe ci robić rzeczy, jest kulawy.

Zasadniczo sposób, w jaki to robię, jest następujący: jeśli uzyskanie rzeczy jest tanie obliczeniowo (lub jeśli istnieje duża szansa, że ​​zostaną znalezione w pamięci podręcznej), to twój styl getStuff () jest w porządku. Jeśli wiadomo, że pobieranie rzeczy jest drogie obliczeniowo, tak drogie, że reklamowanie jego kosztowności jest konieczne w interfejsie, to nie nazwałbym go getStuff (), nazwałbym go wyliczeniemStuff () lub czymś podobnym, aby wskazać, że będzie trochę pracy do zrobienia.

W obu przypadkach sposób, w jaki praca mówi ci, aby robić rzeczy, jest kiepski, ponieważ getStuff () wybuchnie, jeśli loadStuff () nie zostanie wcześniej wywołany, więc w zasadzie chcą, abyś skomplikował interfejs, wprowadzając złożoność kolejności operacji do tego. Kolejność operacji dotyczy w zasadzie najgorszego rodzaju złożoności, jaki mogę sobie wyobrazić.

Logika w gettersach jest w porządku.

Ale pobieranie danych z bazy danych to o wiele więcej niż „logika”. Wiąże się z serią bardzo kosztownych operacji, w których wiele rzeczy może pójść nie tak i w sposób niedeterministyczny. Wahałbym się zrobić to niejawnie w getterze.

Z drugiej strony większość ORM obsługuje leniwe ładowanie kolekcji, co w zasadzie jest dokładnie tym, co robisz.

Myślę, że zgodnie z „Czystym kodem” należy go podzielić na tyle, na ile to możliwe, na coś takiego:

public List<Stuff> getStuff() {
   if (hasStuff()) {
       return stuff;
   }
   loadStuff();
   return stuff;
}

private boolean hasStuff() {
    if (stuff == null) {
       return false;
    }
    if (cacheInvalid()) {
       return false;        
    }
    return true;
} 

private void loadStuff() {
    stuff = getStuffFromDatabase();
}

Oczywiście jest to kompletny nonsens, biorąc pod uwagę, że piękna forma, którą napisałeś, robi dobrze z ułamkiem kodu, który każdy rozumie na pierwszy rzut oka:

public List<Stuff> getStuff() {
   if (stuff == null || cacheInvalid()) {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

Nie powinien to być ból głowy osoby dzwoniącej, jak rzeczy są pod maską, a szczególnie nie powinno być bólem głowy osoby dzwoniącej, aby pamiętać, aby nazywać rzeczy w dowolnej arbitralnej „właściwej kolejności”.

Mówią mi, że w getterach i setterach powinna być jak najmniej logiki.

Musi być tyle logiki, ile potrzeba, aby zaspokoić potrzeby klasy. Moją osobistą preferencją jest jak najmniej, ale przy utrzymywaniu kodu zwykle musisz opuścić oryginalny interfejs z istniejącymi modułami pobierającymi / ustawiającymi, ale wkładaj w to dużo logiki, aby poprawić nowszą logikę biznesową (na przykład „klienci” „getter w środowisku po 911 musi spełniać wymagania„ znać swojego klienta ”i regulamin OFAC , w połączeniu z polityką firmy zabraniającą pojawiania się klientów z niektórych krajów [takich jak Kuba czy Iran]).

W twoim przykładzie wolę twój i nie lubię próbki „wujek bob”, ponieważ wersja „wujek bob” wymaga, aby użytkownicy / opiekunowie pamiętali, aby zadzwonić, loadStuff()zanim zadzwonią getStuff()- jest to przepis na katastrofę, jeśli którykolwiek z twoich opiekunów zapomni (lub gorzej, nigdy nie wiedziałem). Większość miejsc, w których pracowałem w ciągu ostatniej dekady, nadal używa kodu, który ma ponad dziesięć lat, więc łatwość konserwacji jest kluczowym czynnikiem do rozważenia.

Masz rację, twoi koledzy się mylą.

Zapomnij ogólne zasady dotyczące tego, co metoda get powinna, a czego nie powinna robić. Klasa powinna przedstawiać abstrakcję czegoś. Twoja klasa jest czytelna stuff. W Javie konwencjonalne jest stosowanie metod „get” do odczytu właściwości. Miliardy wierszy frameworków zostały napisane w oczekiwaniu na odczyt stuffprzez wywołanie getStuff. Jeśli nazwiesz swoją funkcję fetchStufflub coś innego getStuff, twoja klasa będzie niezgodna ze wszystkimi tymi frameworkami.

Możesz wskazać im Hibernację, gdzie „getStuff ()” może robić bardzo skomplikowane rzeczy i zgłasza wyjątek RuntimeException w przypadku niepowodzenia.

Wygląda na to, że może to być nieco purystyczna debata w porównaniu z aplikacją, na którą może wpływać sposób, w jaki wolisz kontrolować nazwy funkcji. Z przyjętego punktu widzenia wolałbym raczej zobaczyć:

List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

W przeciwieństwie do:

myObject.load();
List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

Lub jeszcze gorzej:

myObject.loadNames();
List<String> names = clientRoster.getNames();
myOjbect.loadEmails();
List<String> emails = clientRoster.getEmails();

Co sprawia, że ​​inny kod jest znacznie bardziej zbędny i trudniejszy do odczytania, ponieważ musisz zacząć brnąć przez wszystkie podobne wywołania. Ponadto wywoływanie funkcji modułu ładującego lub tym podobnych przerywa cały cel nawet używania OOP w tym sensie, że nie jesteś już odciągany od szczegółów implementacji obiektu, z którym pracujesz. Jeśli masz clientRosterobiekt, nie powinieneś przejmować się tym, jak getNamesdziała, tak jak gdybyś musiał zadzwonić loadNames, powinieneś po prostu wiedzieć, że getNamesdaje ci to List<String>z nazwiskami klientów.

Wygląda więc na to, że problem dotyczy bardziej semantyki i najlepszej nazwy dla funkcji, aby uzyskać dane. Jeśli firma (i inne) ma problem z prefiksem geti set, to co powiesz na wywołanie funkcji w taki sposób retrieveNames? Mówi, co się dzieje, ale nie oznacza, że ​​operacja byłaby natychmiastowa, jak można się spodziewać po getmetodzie.

Jeśli chodzi o logikę w metodzie akcesora, ogranicz ją do minimum, ponieważ na ogół implikuje się, że są one natychmiastowe, a tylko zmienna nominalna występuje z tą zmienną. Jednak ogólnie dotyczy to tylko prostych typów, złożonych typów danych (tj. List) Trudniej jest mi właściwie zakapsułkować właściwość i ogólnie używam innych metod interakcji z nimi, w przeciwieństwie do ścisłego mutatora i akcesorium.

Wywołanie gettera powinno wykazywać to samo zachowanie, co czytanie pola:

• Odzyskiwanie wartości powinno być tanie
• Jeśli ustawisz wartość za pomocą settera, a następnie przeczytasz ją za pomocą gettera, wartość powinna być taka sama
• Uzyskanie wartości nie powinno mieć skutków ubocznych
• Nie powinien wyrzucać wyjątku

Moduł pobierający, który wywołuje inne właściwości i metody w celu obliczenia własnej wartości, również implikuje zależność. Na przykład, jeśli twoja właściwość musi być w stanie sama się obliczyć, a wykonanie tej czynności wymaga ustawienia innego elementu członkowskiego, musisz się martwić o przypadkowe odwołania zerowe, jeśli twoja właściwość jest dostępna w kodzie inicjującym, w którym niekoniecznie są ustawione wszyscy członkowie.

Nie oznacza to, że „nigdy nie uzyskuj dostępu do innego elementu, który nie jest polem podkładowym właściwości w module pobierającym”, oznacza to jedynie zwrócenie uwagi na to, co sugerujesz na temat wymaganego stanu obiektu, i jeśli odpowiada to oczekiwanemu kontekstowi ta właściwość będzie dostępna w.

Jednak w dwóch konkretnych przykładach, które podałeś, powód, dla którego wybrałbym jeden z nich, jest zupełnie inny. Twój getter jest inicjowany przy pierwszym dostępie, np. Leniwa inicjalizacja . Zakłada się, że drugi przykład został zainicjowany w pewnym wcześniejszym punkcie, np. Jawna inicjalizacja .

Kiedy dokładnie nastąpi inicjalizacja może, ale nie musi być ważne.

Na przykład może być bardzo bardzo powolny i należy to zrobić podczas etapu ładowania, w którym użytkownik oczekuje opóźnienia, zamiast wydajności nieoczekiwanie odczuwającej problemy, gdy użytkownik po raz pierwszy wyzwala dostęp (tj. Kliknięcie prawym przyciskiem myszy, pojawi się menu kontekstowe, użytkownik już kliknął ponownie prawym przyciskiem myszy).

Czasami jest też oczywisty moment w wykonaniu, w którym występuje wszystko, co może wpłynąć na / wyczyścić wartość buforowanej właściwości. Być może nawet weryfikujesz, czy żadna z zależności się nie zmienia, i wprowadzając wyjątki później. W tej sytuacji sensowne jest także buforowanie wartości w tym momencie, nawet jeśli obliczenia nie są szczególnie drogie, po prostu w celu uniknięcia skomplikowania i trudniejszego mentalnego wykonania kodu.

To powiedziawszy, Lazy Initialization ma wiele sensu w wielu innych sytuacjach. Tak więc, jak to często bywa w programowaniu trudno sprowadzić się do reguły, sprowadza się do konkretnego kodu.

Po prostu zrób to tak, jak powiedział @MikeNakis ... Jeśli tylko dostaniesz te rzeczy, to będzie w porządku ... Jeśli zrobisz coś innego, stwórz nową funkcję, która wykona zadanie i upublicznij.

Jeśli twoja właściwość / funkcja wykonuje tylko to, co mówi jej nazwa, oznacza to, że nie ma już miejsca na komplikacje. Spójność jest kluczowa dla IMO

Osobiście odsłoniłbym wymaganie Stuff za pomocą parametru w konstruktorze i pozwoliłbym, by klasa, która tworzy instancję, wykonała zadanie, aby dowiedzieć się, skąd ona powinna pochodzić. Jeśli rzeczy są zerowe, powinny zwracać null. Wolę nie próbować sprytnych rozwiązań, takich jak oryginał OP, ponieważ jest to łatwy sposób na ukrywanie błędów głęboko w twojej implementacji, gdzie wcale nie jest oczywiste, co może pójść nie tak, gdy coś się zepsuje.

Są ważniejsze kwestie niż tylko „stosowność” tutaj i powinieneś na nich oprzeć swoją decyzję . Główną ważną decyzją jest to, czy chcesz pozwolić ludziom ominąć pamięć podręczną, czy nie.

1. Najpierw zastanów się, czy istnieje sposób na reorganizację kodu, aby wszystkie niezbędne wywołania ładowania i zarządzanie pamięcią podręczną były wykonywane w konstruktorze / inicjatorze. Jeśli jest to możliwe, możesz stworzyć klasę, której niezmiennik pozwala zrobić prostemu getterowi z części 2 z bezpieczeństwem złożonego gettera z części 1. (Scenariusz wygrany-wygrany)

2. Jeśli nie możesz utworzyć takiej klasy, zdecyduj, czy masz kompromis i musisz zdecydować, czy chcesz pozwolić konsumentowi pominąć kod sprawdzający pamięć podręczną, czy nie.

   1. Jeśli ważne jest, aby konsument nigdy nie pomijał kontroli pamięci podręcznej, a nie przeszkadzają temu kary wydajnościowe, należy połączyć czek wewnątrz modułu pobierającego i uniemożliwić konsumentowi wykonanie niewłaściwej czynności.

   2. Jeśli można pominąć sprawdzanie pamięci podręcznej lub bardzo ważne jest, aby zagwarantować wydajność O (1) w module pobierającym, użyj osobnych wywołań.

Jak zapewne zauważyliście, nie jestem wielkim fanem filozofii „czystego kodu”, „podzielenia wszystkiego na małe funkcje”. Jeśli masz kilka funkcji ortogonalnych, które można wywoływać w dowolnej kolejności, ich podzielenie da ci większą moc ekspresji przy niewielkim koszcie. Jeśli jednak funkcje są zależne od kolejności (lub są naprawdę użyteczne tylko w określonej kolejności), to ich podział zwiększa tylko liczbę sposobów wykonywania niewłaściwych czynności, a jednocześnie nie przynosi większych korzyści.

Moim zdaniem Getters nie powinien mieć w sobie dużo logiki. Nie powinny mieć skutków ubocznych i nigdy nie powinieneś otrzymywać od nich wyjątku. Chyba że wiesz, co robisz. Większość moich pobierających nie ma w nich logiki i po prostu idzie na pole. Jednak godnym uwagi wyjątkiem był publiczny interfejs API, który musiał być możliwie najprostszy w użyciu. Miałem więc jednego gettera, który upadłby, gdyby nie został wywołany inny getter. Rozwiązanie? Linia kodu jak var throwaway=MyGetter;w getterze, która od tego zależała. Nie jestem z tego dumny, ale nadal nie widzę czystszego sposobu na zrobienie tego

To wygląda jak odczyt z pamięci podręcznej z leniwym ładowaniem. Jak zauważyli inni, sprawdzanie i ładowanie może należeć do innych metod. Ładowanie może wymagać synchronizacji, aby nie uzyskać dwudziestu wątków jednocześnie ładujących.

Może być właściwe użycie nazwy getCachedStuff()dla modułu pobierającego, ponieważ nie będzie on miał spójnego czasu wykonania.

W zależności od tego, jak cacheInvalid()działa procedura, sprawdzenie wartości null może nie być konieczne. Nie spodziewałbym się, że pamięć podręczna będzie poprawna, jeśli stuffnie zostanie zapełniona z bazy danych.

Główną logiką, jakiej spodziewałbym się w modułach pobierających, które zwracają listę, jest logika zapewniająca, że ​​lista jest niezmodyfikowana. W obecnej postaci oba przykłady mogą przerwać enkapsulację.

coś jak:

public List<Stuff> getStuff()
{
    return Collections.unmodifiableList(stuff);
}

jeśli chodzi o buforowanie w module pobierającym, myślę, że byłoby to w porządku, ale może mnie kusić, aby odejść od logiki pamięci podręcznej, jeśli zbudowanie pamięci podręcznej zajmie dużo czasu. tzn. to zależy.

W zależności od dokładnego przypadku użycia lubię rozdzielać pamięć podręczną na osobną klasę. Stwórz StuffGetterinterfejs, zaimplementuj StuffComputerobliczenia i zawiń go w obiekcie StuffCacher, który jest odpowiedzialny za dostęp do pamięci podręcznej lub przekazywanie wywołań do tego StuffComputer, co otacza.

interface StuffGetter {
     public List<Stuff> getStuff();
}

class StuffComputer implements StuffGetter {
     public List<Stuff> getStuff() {
         getStuffFromDatabase()
     }
}

class StuffCacher implements StuffGetter {
     private stuffComputer; // DI this
     private Cache<List<Stuff>> cache = new Cache<>();

     public List<Stuff> getStuff() {
         if cache.hasStuff() {
             return cache.getStuff();
         }

         List<Stuffs> stuffs = stuffComputer.getStuff();
         cache.store(stuffs);
         return stuffs;
     }
}

Ten projekt pozwala łatwo dodawać buforowanie, usuwać buforowanie, zmieniać podstawową logikę wyprowadzania (np. Uzyskiwanie dostępu do bazy danych w porównaniu do zwracania próbnych danych) itp. Jest to trochę kłopotliwe, ale warto poświęcić czas na wystarczająco zaawansowane projekty.

IMHO jest bardzo proste, jeśli używasz projektu na podstawie umowy. Zdecyduj, co powinien zapewnić twój moduł pobierający i po prostu odpowiednio koduj (prosty kod lub złożona logika, która może być gdzieś zaangażowana lub delegowana).