Lepiej mieć 2 metody o wyraźnym znaczeniu, czy tylko 1 metodę podwójnego zastosowania?

30

Aby uprościć interfejs, czy lepiej nie mieć tej getBalance()metody? Przejście 0do charge(float c);daje taki sam wynik:

public class Client {
    private float bal;
    float getBalance() { return bal; }
    float charge(float c) {
        bal -= c;
        return bal;
    }
}

Może zanotuj javadoc? Lub po prostu pozostawić użytkownikowi klasy, aby dowiedzieć się, jak uzyskać równowagę?

David
źródło
26
Hojnie zakładam, że to przykład, przykładowy kod i że nie używasz matematyki zmiennoprzecinkowej do przechowywania wartości pieniężnych. W przeciwnym razie musiałbym zdobyć całą kaznodzieję. :-)
corsiKa
1
@corsiKa nah. To tylko przykład. Ale tak, użyłbym liczby zmiennoprzecinkowej do reprezentowania pieniędzy w prawdziwej klasie ... Dzięki za przypomnienie mi o niebezpieczeństwach liczb zmiennoprzecinkowych.
David
10
Niektóre języki rozróżniają mutatory i akcesoria, aby uzyskać dobry efekt. Byłoby okropnie denerwujące, że nie można uzyskać równowagi w instancji, która jest dostępna tylko jako stała!
JDługosz

Odpowiedzi:

90

Wydaje się, że sugerujesz, że złożoność interfejsu jest mierzona liczbą elementów, które ma (w tym przypadku metod). Wielu twierdziło, że konieczność pamiętania, że chargemetoda może być wykorzystana do przywrócenia równowagi a Clientdodaje znacznie większej złożoności niż posiadanie dodatkowego elementu getBalancemetody. Wyjaśnienie rzeczy jest znacznie prostsze, szczególnie do tego stopnia, że ​​nie pozostawia niejednoznaczności, niezależnie od większej liczby elementów interfejsu.

Poza tym dzwonienie charge(0)narusza zasadę najmniejszego zdziwienia , znaną również jako metryka WTF na minutę (z Clean Code, zdjęcie poniżej), co utrudnia nowym członkom zespołu (lub obecnym, po pewnym czasie od kodu), aż rozumieją, że połączenie jest rzeczywiście wykorzystywane do uzyskania równowagi. Pomyśl, jak zareagowaliby inni czytelnicy:

wprowadź opis zdjęcia tutaj

Ponadto podpis chargemetody jest niezgodny z wytycznymi wykonywania jednej i tylko jednej rzeczy oraz rozdzielania poleceń i zapytań , ponieważ powoduje, że obiekt zmienia swój stan, a jednocześnie zwraca nową wartość.

Podsumowując, uważam, że najprostszym interfejsem w tym przypadku byłoby:

public class Client {
  private float bal;
  float getBalance() { return bal; }
  void charge(float c) { bal -= c; }
}
MichelHenrich
źródło
25
Sprawa dotycząca rozdzielania zapytań od poleceń jest niezwykle ważna, IMO. Wyobraź sobie, że jesteś nowym programistą w tym projekcie. Przeglądając kod, od razu widać, że getBalance()zwraca pewną wartość i niczego nie modyfikuje. Z drugiej strony charge(0)wygląda na to, że prawdopodobnie coś modyfikuje ... może wysyła zdarzenie PropertyChanged? A co to jest, poprzednia lub nowa wartość? Teraz musisz to sprawdzić w dokumentacji i zmarnować siłę roboczą, czego można by uniknąć, stosując jaśniejszą metodę.
Mag Xy,
19
Ponadto użycie charge(0)jako sposobu na uzyskanie równowagi, ponieważ okazuje się, że nie ma żadnego efektu, odsyła cię teraz do tych szczegółów implementacji; Mogę łatwo wyobrazić sobie przyszły wymóg, w którym śledzenie liczby ładunków staje się istotne, w którym to momencie zaszywanie bazy kodu ładunkami, które tak naprawdę nie są ładunkami, staje się problemem. Powinieneś zapewnić elementy interfejsu, które oznaczają rzeczy, które muszą zrobić Twoi klienci, a nie takie, które po prostu robią to, czego potrzebują Twoi klienci.
Ben
12
Ostrzeżenie CQS niekoniecznie jest odpowiednie. W przypadku charge()metody, jeśli metoda ta jest atomowa w systemie współbieżnym, może być właściwe zwrócenie nowej lub starej wartości.
Dietrich Epp,
3
Uważam, że twoje dwa cytaty dotyczące CQRS są wyjątkowo nieprzekonujące. Na przykład ogólnie lubię pomysły Meyera, ale patrzenie na typ zwracany przez funkcję, aby stwierdzić, czy coś mutuje, jest arbitralne i nieufne. Wiele współbieżnych lub niezmiennych struktur danych wymaga mutacji + zwrotów. Jak dołączysz do ciągu bez naruszania CQRS? Czy masz jakieś lepsze cytaty?
user949300
6
Prawie żaden kod nie jest zgodny z separacją zapytań poleceń. Nie jest to idiomatyczne w żadnym popularnym języku obiektowym i jest naruszane przez wbudowane interfejsy API każdego języka, którego kiedykolwiek używałem. Opisanie tego jako „najlepszej praktyki” wydaje się więc dziwne; czy potrafisz wymienić choćby jeden projekt oprogramowania, który faktycznie jest zgodny z tym schematem?
Mark Amery
28

IMO, zastępowanie getBalance()w charge(0)całej aplikacji nie jest uproszczeniem. Tak, jest mniej wierszy, ale zaciemnia znaczenie charge()metody, co może potencjalnie powodować bóle głowy w dół linii, gdy ty lub ktoś inny musisz ponownie odwiedzić ten kod.

Chociaż mogą dać ten sam wynik, uzyskanie salda konta nie jest równoznaczne z opłatą zerową, więc prawdopodobnie najlepiej byłoby rozdzielić swoje obawy. Na przykład, jeśli kiedykolwiek musiałeś zmodyfikować charge()rejestrowanie za każdym razem, gdy istnieje transakcja na koncie, masz teraz problem i i tak musisz oddzielić tę funkcję.

Matt Dalzell
źródło
13

Ważne jest, aby pamiętać, że Twój kod powinien być samodokumentujący. Kiedy dzwonię charge(x), oczekuję xobciążenia. Informacje na temat równowagi są drugorzędne. Co więcej, mogę nie wiedzieć, jak charge()jest zaimplementowany, kiedy go nazywam, i na pewno nie będę wiedział, jak zostanie zaimplementowany jutro. Na przykład rozważ tę potencjalną przyszłą aktualizację, aby charge():

float charge(float c) {
    lockDownUserAccountUntilChargeClears();
    bal -= c;
    Unlock();
    return bal;
}

Nagle używanie w charge()celu uzyskania równowagi nie wygląda tak dobrze.

David mówi Przywróć Monikę
źródło
Tak! To dobry punkt, który chargema znacznie większą wagę.
Deduplicator
2

Używanie w charge(0);celu uzyskania równowagi jest złym pomysłem: pewnego dnia ktoś może dodać tam kod, aby zarejestrować naliczane opłaty, nie zdając sobie sprawy z innego wykorzystania funkcji, a następnie za każdym razem, gdy ktoś otrzyma saldo, zostanie zarejestrowany jako opłata. (Istnieją na to sposoby, takie jak warunkowe stwierdzenie, które mówi coś takiego:

if (c > 0) {
    // make and log charge
}
return bal;

ale polegają one na tym, że programista umie je wdrożyć, czego nie zrobi, jeśli nie będzie od razu oczywiste, że są one konieczne.

Krótko mówiąc: nie polegaj ani na swoich użytkownikach, ani na programistach, charge(0);którzy zdają sobie sprawę, że jest to właściwy sposób na uzyskanie równowagi, ponieważ chyba że istnieje dokumentacja, której nie można przegapić, to szczerze mówiąc, która wygląda na najbardziej przerażający sposób na uzyskanie równowagi możliwy.

Micheal Johnson
źródło
0

Wiem, że jest wiele odpowiedzi, ale innym powodem charge(0)jest to, że zwykła literówka charge(9)spowoduje zmniejszenie salda klienta za każdym razem, gdy chcesz uzyskać równowagę. Jeśli masz dobre testy jednostkowe, możesz być w stanie zminimalizować to ryzyko, ale jeśli nie będziesz sumienny przy każdym połączeniu do chargeciebie, możesz mieć ten wypadek.

Shaz
źródło
-2

Chciałbym wspomnieć o szczególnym przypadku, gdzie to byłoby sensu mieć mniej, bardziej uniwersalnego sposoby: jeśli istnieje wiele polimorfizmu, czyli wielu implementacji tego interfejsu ; szczególnie jeśli te implementacje są w osobnym kodzie, którego nie można zaktualizować synchronicznie (interfejs jest zdefiniowany przez bibliotekę).

W takim przypadku uproszczenie zadania pisania każdej implementacji jest znacznie cenniejsze niż przejrzystość jej użycia, ponieważ ta pierwsza pozwala uniknąć błędów związanych z naruszeniem umowy (dwie metody są ze sobą niespójne), natomiast druga szkodzi jedynie czytelności, co może być odzyskane za pomocą funkcji pomocniczej lub metody nadklasy definiującej getBalancew kategoriach charge.

(Jest to wzorzec projektowy, dla którego nie pamiętam konkretnej nazwy: definiowanie złożonego interfejsu przyjaznego dla dzwoniącego w kategoriach minimalnego interfejsu przyjaznego implementatorowi. W klasycznym Mac OS minimalny interfejs do operacji rysowania nazywano „wąskim gardłem” ale to nie wydaje się popularne.)

Jeśli tak nie jest (jest kilka implementacji lub dokładnie jedna), wówczas rozdzielenie metod dla jasności i umożliwienie prostego dodania zachowania istotnego dla ładunków niezerowych charge()ma sens.

Kevin Reid
źródło
1
Rzeczywiście miałem przypadki, w których wybrano bardziej minimalny interfejs, aby ułatwić pełne testowanie zasięgu. Ale to niekoniecznie jest narażone na użytkownika klasy. Np. Wstawianie , dołączanie i usuwanie mogą być prostymi opakowaniami ogólnego zamiennika, który ma dobrze zbadane i przetestowane wszystkie ścieżki sterowania.
JDługosz
Widziałem taki interfejs API. Korzysta z niego dzielnik Lua 5.1+. Udostępniasz jedną funkcję i na podstawie przekazywanych parametrów decydujesz, czy próbuje ona przydzielić nową pamięć, cofnąć przydział pamięci, czy ponownie przydzielić pamięć ze starej pamięci. Pisanie takich funkcji jest bolesne . Wolałbym, żeby Lua właśnie dała ci dwie funkcje, jedną do przydzielenia, a drugą do zwolnienia.
Nicol Bolas,
@NicolBolas: I żaden do realokacji? W każdym razie ta dodatkowa „korzyść” jest prawie taka sama, jak reallocw niektórych systemach.
Deduplicator
@Deduplicator: alokacja kontra realokacja jest ... OK. Umieszczanie ich w tej samej funkcji nie jest wspaniałe, ale nie jest tak złe, jak dezalokacja w tej samej funkcji. Jest to również łatwe do rozróżnienia.
Nicol Bolas,
Powiedziałbym, że połączenie dezalokacji z (re) alokacją jest szczególnie złym przykładem tego rodzaju interfejsu. (Dezalokacja ma zupełnie inną umowę: nie skutkuje prawidłowym wskaźnikiem.)
Kevin Reid