Czy dodanie typu zwrotu do metody aktualizacji narusza „zasadę pojedynczej odpowiedzialności”?

Mam metodę, która aktualizuje dane pracowników w bazie danych. EmployeeKlasa jest niezmienna, więc „aktualizowanie” środki obiekt rzeczywiście instancji nowego obiektu.

Chcę, aby Updatemetoda zwróciła nowe wystąpienie Employeez zaktualizowanymi danymi, ale ponieważ teraz mogę powiedzieć, że odpowiedzialnością metody jest aktualizacja danych pracowników i pobranie z bazy danych nowego Employeeobiektu , czy narusza to zasadę pojedynczej odpowiedzialności ?

Sam rekord DB jest aktualizowany. Następnie tworzy się nowy obiekt reprezentujący ten rekord.

Jak w przypadku każdej reguły, myślę, że ważną rzeczą tutaj jest rozważenie celu reguły, ducha, a nie zanurzenie się w analizowaniu dokładnego sformułowania reguły w jakimś podręczniku i tego, jak zastosować ją w tym przypadku. Nie musimy podchodzić do tego jak prawnicy. Celem tych zasad jest pomoc w pisaniu lepszych programów. To nie jest tak, że celem pisania programów jest przestrzeganie zasad.

Celem zasady pojedynczej odpowiedzialności jest ułatwienie zrozumienia i utrzymania programów poprzez uczynienie każdej funkcji jedną samodzielną, spójną rzeczą.

Na przykład napisałem kiedyś funkcję, którą nazwałem „checkOrderStatus”, która określa, czy zamówienie jest w toku, jest wysyłane, zamawia z powrotem, cokolwiek, i zwraca kod wskazujący, który. Potem pojawił się inny programista i zmodyfikował tę funkcję, aby zaktualizować ilość dostępną w momencie wysyłki zamówienia. To poważnie naruszyło zasadę pojedynczej odpowiedzialności. Inny programista czytający ten kod później zobaczy nazwę funkcji, zobaczy, w jaki sposób została użyta wartość zwracana, i może nigdy nie podejrzewać, że dokonał aktualizacji bazy danych. Ktoś, kto musiał uzyskać status zamówienia bez aktualizacji dostępnej ilości, znalazłby się w niezręcznej sytuacji: czy powinien napisać nową funkcję, która powiela część statusu zamówienia? Dodaj flagę, aby powiedzieć, czy wykonać aktualizację bazy danych? Itp.

Z drugiej strony nie wybrałbym nic, co stanowi „dwie rzeczy”. Niedawno napisałem funkcję, która wysyła informacje o kliencie z naszego systemu do systemu naszego klienta. Ta funkcja dokonuje ponownego formatowania danych, aby spełnić ich wymagania. Na przykład w naszej bazie danych mamy pola, które mogą mieć wartość null, ale nie pozwalają one na wartości null, więc musimy wypełnić tekst zastępczy „nieokreślony” lub zapomnę dokładnych słów. Prawdopodobnie ta funkcja wykonuje dwie czynności: ponownie sformatuje dane ORAZ je wyśle. Ale bardzo celowo umieszczam to w jednej funkcji zamiast „reformatować” i „wysyłać”, ponieważ nie chcę nigdy, nigdy wysyłać bez formatowania. Nie chcę, aby ktoś napisał nowe połączenie i nie zdawał sobie sprawy, że musi zadzwonić do sformatowania, a następnie wysłać.

W twoim przypadku zaktualizuj bazę danych i zwróć obraz zapisanego rekordu, wydają się dwiema rzeczami, które mogą się ze sobą logicznie i nieuchronnie połączyć. Nie znam szczegółów twojej aplikacji, więc nie mogę definitywnie stwierdzić, czy to dobry pomysł, czy nie, ale brzmi to realistycznie.

Jeśli tworzysz w pamięci obiekt, który przechowuje wszystkie dane rekordu, wykonujesz wywołania bazy danych, aby je zapisać, a następnie zwracasz obiekt, ma to sens. Masz przedmiot w swoich rękach. Dlaczego nie oddasz go z powrotem? Jeśli obiekt nie został zwrócony, w jaki sposób osoba dzwoniąca mogłaby go zdobyć? Czy musiałby przeczytać bazę danych, aby uzyskać właśnie napisany obiekt? To wydaje się raczej nieefektywne. Jak znalazłby płytę? Czy znasz klucz podstawowy? Jeśli ktoś deklaruje, że „legalne” jest, aby funkcja zapisu zwróciła klucz podstawowy, aby umożliwić ponowne odczytanie rekordu, dlaczego po prostu nie zwrócić całego rekordu, abyś nie musiał? Co za różnica?

Z drugiej strony, jeśli tworzenie obiektu różni się znacznie od zapisu rekordu bazy danych, a osoba dzwoniąca może chcieć wykonać zapis, ale nie utworzyć obiektu, może to być marnotrawstwem. Jeśli osoba dzwoniąca może chcieć obiektu, ale nie wykonuje zapisu, musisz podać inny sposób uzyskania obiektu, co może oznaczać napisanie zbędnego kodu.

Myślę jednak, że scenariusz 1 jest bardziej prawdopodobny, więc powiedziałbym, że prawdopodobnie nie ma problemu.

Koncepcja SRP polega na tym, aby moduły nie robiły 2 różnych rzeczy, a ich indywidualne wykonanie zapewnia lepszą konserwację i mniej spaghetti na czas. Jak mówi SRP „jeden powód do zmiany”.

W twoim przypadku, jeśli masz procedurę, która „aktualizuje i zwraca zaktualizowany obiekt”, nadal zmieniasz obiekt raz - dając 1 powód do zmiany. To, że zwrócisz obiekt z powrotem, nie ma go ani tu, ani tam, nadal operujesz na tym pojedynczym obiekcie. Kod odpowiada za jedną i tylko jedną rzecz.

SRP tak naprawdę nie polega na próbie zredukowania wszystkich operacji do jednego połączenia, ale na zmniejszeniu tego, na czym operujesz i jak na tym operujesz. Tak więc pojedyncza aktualizacja, która zwraca zaktualizowany obiekt, jest w porządku.

Jak zawsze jest to kwestia stopnia. SRP powinien powstrzymać cię przed napisaniem metody, która pobiera rekord z zewnętrznej bazy danych, wykonuje na nim szybką transformację Fouriera i aktualizuje rejestr statystyk globalnych z wynikiem. Myślę, że prawie wszyscy zgodziliby się, że te rzeczy powinny być wykonane różnymi metodami. Postulowanie jednej odpowiedzialności za każdą metodę jest po prostu najbardziej ekonomicznym i niezapomnianym sposobem na osiągnięcie tego.

Na drugim końcu spektrum znajdują się metody, które dostarczają informacji o stanie obiektu. Typowy podaje isActivetę informację jako jedyną odpowiedzialność. Prawdopodobnie wszyscy zgadzają się, że to jest w porządku.

Teraz niektórzy rozszerzają tę zasadę do tego stopnia, że ​​rozważają zwrócenie flagi sukcesu inną odpowiedzialność niż wykonanie akcji, której sukces jest zgłaszany. Przy bardzo ścisłej interpretacji jest to prawda, ale ponieważ alternatywą byłoby wywołanie drugiej metody w celu uzyskania statusu powodzenia, co komplikuje osobę wywołującą, wielu programistów doskonale sobie radzi ze zwracaniem kodów sukcesu z metody wywołującej skutki uboczne.

Zwrot nowego obiektu to kolejny krok na drodze do wielu obowiązków. Wymaganie od dzwoniącego wykonania drugiego wywołania dla całego obiektu jest nieco bardziej uzasadnione niż wymaganie drugiego wywołania tylko w celu sprawdzenia, czy pierwsze się powiodło, czy nie. Mimo to wielu programistów uważa, że ​​zwrócenie wyniku aktualizacji jest całkowicie w porządku. Chociaż można to rozumieć jako dwie nieco odmienne obowiązki, z pewnością nie jest to jedno z rażących nadużyć, które zainspirowały zasadę na początek.

Czy narusza zasadę jednolitej odpowiedzialności?

Niekoniecznie. Jeśli tak, narusza to zasadę rozdzielania poleceń i zapytań .

Odpowiedzialność jest

zaktualizuj dane pracownika

z domyślnym zrozumieniem, że odpowiedzialność ta obejmuje status operacji; np. jeśli operacja się nie powiedzie, zgłoszony zostanie wyjątek, jeśli się powiedzie, pracownik aktualizacji zostanie zwrócony itp.

Ponownie, wszystko to jest kwestią stopnia i subiektywnej oceny.

A co z rozdzielaniem zapytań i poleceń?

Cóż, ta zasada istnieje, ale zwracanie wyniku aktualizacji jest w rzeczywistości bardzo powszechne.

(1) Java Set#add(E)dodaje element i zwraca poprzednie włączenie do zestawu.

if (visited.add(nodeToVisit)) {
    // perform operation once
}

Jest to bardziej wydajne niż alternatywa CQS, która może wymagać wykonania dwóch wyszukiwań.

if (!visited.contains(nodeToVisit)) {
    visited.add(nodeToVisit);
    // perform operation once
}

(2) Porównywanie i zamiana , pobieranie i dodawanie oraz testowanie i zestaw to typowe operacje podstawowe, które umożliwiają istnienie programowania współbieżnego. Te wzorce pojawiają się często, od instrukcji procesora niskiego poziomu do współbieżnych kolekcji wysokiego poziomu.

Pojedyncza odpowiedzialność polega na tym, że klasa nie musi się zmieniać z więcej niż jednego powodu.

Na przykład pracownik ma listę numerów telefonów. Gdy zmieni się sposób obsługi numerów telefonów (możesz dodać kody połączeń krajowych), nie powinno to wcale zmieniać klasy pracowników.

Nie chciałbym, aby klasa pracowników musiała wiedzieć, jak zapisuje się w bazie danych, ponieważ zmieniłaby się wtedy w przypadku zmian w pracownikach i zmian w sposobie przechowywania danych.

Podobnie nie powinno być metody CalculateSalary w klasie pracowników. Nieruchomość za wynagrodzenie jest w porządku, ale obliczenia podatku itp. Należy dokonać gdzie indziej.

Ale to, że metoda aktualizacji zwraca to, co właśnie zaktualizowała, jest w porządku.

Wrt. konkretny przypadek:

Employee Update(Employee, name, password, etc) (właściwie korzystam z Buildera, ponieważ mam wiele parametrów).

To wydajeUpdate Metoda zajmuje istniejąca Employeejako pierwszy parametr do zidentyfikowania (?) Istniejącego pracownika oraz zestaw parametrów do zmian na tej pracownika.

Myślę , że można to zrobić czystsze. Widzę dwa przypadki:

(a) Employee faktycznie zawiera bazę danych / unikalny identyfikator, dzięki któremu zawsze można ją zidentyfikować w bazie danych. (Oznacza to, że nie potrzebujesz całego zestawu rekordów, aby znaleźć go w bazie danych.

W takim przypadku wolałbym void Update(Employee obj)metodę, która po prostu znajduje istniejący rekord według identyfikatora, a następnie aktualizuje pola z przekazywanego obiektu. A możevoid Update(ID, EmployeeBuilder values)

Odmianą tego, którą uznałem za przydatną, jest posiadanie void Put(Employee obj)metody wstawiania lub aktualizacji, w zależności od tego, czy rekord (według identyfikatora) istnieje.

(b) Pełne istniejący zapis jest potrzebny do DB odnośnika, w tym przypadku może to jeszcze więcej sensu posiadać: void Update(Employee existing, Employee newData).

O ile widzę tutaj, naprawdę powiedziałbym, że odpowiedzialność za zbudowanie nowego obiektu (lub wartości podobiektowych) do przechowywania i faktycznego przechowywania jest ortogonalna, więc oddzieliłbym je.

Wspomniane współbieżne wymagania w innych odpowiedziach (atomowe ustawianie i pobieranie / porównywanie-zamiana itp.) Nie stanowiły problemu w kodzie DB, nad którym pracowałem do tej pory. Kiedy rozmawiam z DB, myślę, że należy to załatwić normalnie na poziomie transakcji, a nie na poziomie pojedynczego wyciągu. (Nie oznacza to, że może nie istnieć projekt, w którym jestem „atomowy”, Employee[existing data] Update(ID, Employee newData)nie może mieć sensu, ale w przypadku dostępu do bazy danych nie jest to coś, co normalnie widzę.)

Jak dotąd wszyscy tutaj mówią o zajęciach. Ale pomyśl o tym z perspektywy interfejsu.

Jeśli metoda interfejsu deklaruje typ zwrotu i / lub niejawną obietnicę dotyczącą wartości zwracanej, każda implementacja musi zwrócić zaktualizowany obiekt.

Możesz więc zapytać:

• Czy możesz pomyśleć o możliwych implementacjach, które nie chcą zawracać sobie głowy zwracaniem nowego obiektu?
• Czy możesz pomyśleć o komponentach zależnych od interfejsu (poprzez wstrzyknięcie instancji), które nie potrzebują zaktualizowanego pracownika jako wartości zwracanej?

Pomyśl także o próbnych obiektach do testów jednostkowych. Oczywiście łatwiej będzie kpić bez wartości zwracanej. Komponenty zależne są łatwiejsze do przetestowania, jeśli ich wstrzykiwane zależności mają prostsze interfejsy.

Na podstawie tych rozważań możesz podjąć decyzję.

A jeśli później tego pożałujesz, nadal możesz wprowadzić drugi interfejs z adapterami między nimi. Oczywiście takie dodatkowe interfejsy zwiększają złożoność kompozycji, więc jest to kompromis.

Twoje podejście jest w porządku. Niezmienność jest mocnym twierdzeniem. Chciałbym tylko zapytać: czy jest jakieś inne miejsce, w którym budujesz obiekt. Jeśli twój obiekt nie był niezmienny, musiałbyś odpowiedzieć na dodatkowe pytania, ponieważ wprowadzono „Stan”. Zmiana stanu obiektu może nastąpić z różnych powodów. W takim razie powinieneś znać swoje sprawy i nie powinny one być zbędne ani podzielone.