Oto często spotykany problem: Niech będzie projekt sklepu internetowego, który ma klasę produktu. Chcę dodać funkcję, która pozwala użytkownikom publikować recenzje w produkcie. Mam więc klasę Review, która odwołuje się do produktu. Teraz potrzebuję metody, która wyświetla wszystkie recenzje produktu. Istnieją dwie możliwości:
(ZA)
public class Product {
...
public Collection<Review> getReviews() {...}
}
(B)
public class Review {
...
static public Collection<Review> forProduct( Product product ) {...}
}
Spoglądając na kod, wybrałbym (A): Nie jest statyczny i nie potrzebuje parametru. Jednak wyczuwam, że (A) narusza zasadę pojedynczej odpowiedzialności (SRP) i zasadę otwartego i zamkniętego (OCP), podczas gdy (B) nie:
(SRP) Kiedy chcę zmienić sposób zbierania recenzji produktu, muszę zmienić klasę produktu. Ale powinien istnieć tylko jeden powód, dla którego należy zmienić klasę produktu. I to z pewnością nie są recenzje. Jeśli spakuję każdą funkcję, która ma coś wspólnego z produktami w produkcie, wkrótce zostanie stuknięta.
(OCP) Muszę zmienić klasę produktu, aby rozszerzyć ją o tę funkcję. Myślę, że narusza to część zasady „Zamknięci na zmiany”. Zanim dostałem prośbę klienta o wdrożenie recenzji, uznałem Produkt za gotowy i „zamknąłem” go.
Co jest ważniejsze: przestrzeganie zasad SOLID lub prostszy interfejs?
Czy też robię tu coś złego?
Wynik
Wow, dziękuję za wszystkie twoje wspaniałe odpowiedzi! Trudno wybrać jedną jako oficjalną odpowiedź.
Pozwól, że streszczę główne argumenty z odpowiedzi:
- pro (A): OCP nie jest prawem i liczy się również czytelność kodu.
- pro (A): relacja bytu powinna być możliwa do nawigacji. Obie klasy mogą wiedzieć o tym związku.
- pro (A) + (B): wykonaj oba i deleguj w (A) na (B), aby prawdopodobieństwo zmiany produktu było mniejsze.
- pro (C): umieść metody wyszukiwania w trzeciej klasie (usłudze), gdzie nie jest statyczna.
- contra (B): utrudnia drwiny w testach.
Przyczyniło się kilka dodatkowych rzeczy, które włożyły moje uczelnie w pracy:
- pro (B): nasz framework ORM może automatycznie wygenerować kod dla (B).
- pro (A): ze względów technicznych naszej struktury ORM konieczna będzie zmiana „zamkniętego” bytu w niektórych przypadkach, niezależnie od tego, dokąd trafi szukający. W każdym razie i tak nie zawsze będę w stanie pozostać przy SOLID.
- contra (C): za dużo zamieszania ;-)
Wniosek
Używam zarówno (A) + (B) z delegacją do mojego bieżącego projektu. Jednak w środowisku zorientowanym na usługi skorzystam z (C).
źródło
Assert(5 = Math.Abs(-5));
Abs()
nie jest problemem, testowanie czegoś, co od niego zależy. Nie masz szwu do izolowania zależnego testowanego kodu (CUT), aby użyć makiety. Oznacza to, że nie można go przetestować jako jednostki atomowej, a wszystkie testy stają się testami integracyjnymi, które logikę jednostki testowej. Błąd w teście może być w CUT lub wAbs()
(lub jego zależnym kodzie) i usuwa zalety diagnostyczne testów jednostkowych.Odpowiedzi:
Interfejs w stosunku do SOLID
Nie wykluczają się one wzajemnie. Interfejs powinien najlepiej wyrażać naturę modelu biznesowego w kategoriach modelu biznesowego. Zasady SOLID to podstawa do maksymalizacji możliwości utrzymania kodu obiektowego (i mam na myśli „utrzymanie” w najszerszym tego słowa znaczeniu). Pierwszy obsługuje wykorzystanie modelu biznesowego i manipuluje nim, a drugi optymalizuje obsługę kodu.
Zasada otwarta / zamknięta
„nie dotykaj tego!” jest zbyt uproszczoną interpretacją. Zakładając, że mamy na myśli, że „klasa” jest arbitralna i niekoniecznie słuszna. OCP oznacza raczej, że zaprojektowałeś swój kod w taki sposób, że modyfikacja jego zachowania nie wymaga (nie powinna) wymagać bezpośredniej modyfikacji istniejącego, działającego kodu. Ponadto, nie dotykanie kodu w pierwszej kolejności jest idealnym sposobem na zachowanie integralności istniejących interfejsów; Moim zdaniem jest to znacząca konsekwencja OCP.
Wreszcie widzę OCP jako wskaźnik istniejącej jakości projektu. Jeśli zbyt często łamię otwarte klasy (lub metody) i / lub bez naprawdę solidnych (ha, ha) powodów, może to oznaczać, że mam zły projekt (i / lub nie mam umie kodować OO).
Daj z siebie wszystko. Mamy przy sobie zespół lekarzy
Jeśli analiza wymagań mówi, że musisz wyrazić relację przeglądu produktu z obu perspektyw, zrób to.
Dlatego Wolfgang, możesz mieć dobry powód do modyfikowania istniejących klas. Biorąc pod uwagę nowe wymagania, jeśli Recenzja jest teraz podstawową częścią Produktu, jeśli każde rozszerzenie Produktu wymaga Oceny, jeśli to czyni odpowiednio dostosowuje kod klienta, należy zintegrować go z Produktem.
źródło
SOLID to wytyczne, więc wpływaj na podejmowanie decyzji, a nie narzucaj je.
Jedną z rzeczy, o których należy pamiętać przy stosowaniu metod statycznych, jest ich wpływ na testowalność .
Testowanie
forProduct(Product product)
nie stanowi problemu.Będzie testowanie czegoś, co od tego zależy.
Nie będziesz mieć szwu do izolowania zależnego testowanego kodu (CUT), aby użyć makiety, ponieważ gdy aplikacja jest uruchomiona, metody statyczne koniecznie istnieją.
Niech będzie metoda zwana
CUT()
tymi wywołaniamiforProduct()
Jeśli
forProduct()
tostatic
nie można przetestowaćCUT()
jako jednostki atomowe i wszystkich testów stać testy integracyjne, że logika jednostki.Niepowodzenie testu CUT może być spowodowane problemem w
CUT()
lub wforProduct()
(lub dowolnym jego zależnym kodzie), który usuwa zalety diagnostyczne testów jednostkowych.Zobacz ten znakomity post na blogu, aby uzyskać bardziej szczegółowe informacje: http://googletesting.blogspot.com/2008/12/static-methods-are-death-to-testability.html
Może to prowadzić do frustracji wynikającej z nieudanych testów oraz rezygnacji z dobrych praktyk i korzyści, które ich otaczają.
źródło
If forProduct() is static you can't test CUT() as an atomic unit and all of your tests become integration tests that test unit logic.
- Uważam, że zarówno Javascript, jak i Python pozwalają na zastąpienie / wyśmiewanie metod statycznych. Nie jestem jednak w 100% pewien.static
, symulujesz go za pomocą zamknięcia i wzoru singletonu. Czytając w Pythonie (zwłaszcza stackoverflow.com/questions/893015/... ), musisz odziedziczyć i rozszerzyć. Przesłanianie nie jest kpiną; wygląda na to, że nadal nie masz szwu do testowania kodu jako jednostki atomowej.Jeśli uważasz, że Produkt jest właściwym miejscem do znalezienia recenzji produktu, zawsze możesz dać mu klasę pomocy, aby wykonał za niego zadanie. (Możesz to stwierdzić, ponieważ Twoja firma nigdy nie będzie mówić o recenzji, z wyjątkiem produktu).
Na przykład pokusiłbym się o wstrzyknięcie czegoś, co pełniło rolę aportera recenzji. Prawdopodobnie dałbym mu interfejs
IRetrieveReviews
. Możesz umieścić to w konstruktorze produktu (Dependency Injection). Jeśli chcesz zmienić sposób opinie są pobierane można to zrobić łatwo poprzez wstrzyknięcie inny współpracownik -TwitterReviewRetriever
alboAmazonReviewRetriever
alboMultipleSourceReviewRetriever
albo cokolwiek innego, czego potrzebujesz.Obaj ponoszą teraz jedną odpowiedzialność (bycie odpowiednio dla wszystkich rzeczy związanych z produktem i pobieranie recenzji), a w przyszłości zachowanie produktu w odniesieniu do recenzji może być modyfikowane bez faktycznej zmiany produktu (możesz go rozszerzyć jak
ProductWithReviews
gdyby naprawdę chciał być pedantyczny o swoich SOLID zasad, ale to byłoby wystarczająco dobre dla mnie).źródło
IRetrieveReviews
sprawia, że nie jest on zorientowany na usługi - nie determinuje, co otrzyma recenzje, w jaki sposób i kiedy. Może to usługa z wieloma metodami dla takich rzeczy. Może to klasa, która robi to jedno. Może to repozytorium lub wysyła żądanie HTTP do serwera. Nie wiesz Nie powinieneś wiedzieć. O to chodzi.Miałbym klasę ProductsReview. Mówisz, że Recenzja i tak jest nowa. To nie znaczy, że może to być po prostu cokolwiek. Musi mieć tylko jeden powód do zmiany. Jeśli zmienisz sposób uzyskiwania recenzji z jakiegokolwiek powodu, musisz zmienić klasę recenzji.
To nie jest poprawne.
Umieszczasz metodę statyczną w klasie Review, ponieważ ... dlaczego? Czy nie o to walczysz? Czy to nie cały problem?
Więc nie rób tego. Stwórz klasę, której wyłączną odpowiedzialnością jest uzyskiwanie recenzji produktów. Następnie możesz podklasować go do ProductReviewsByStartRating cokolwiek. Lub podklasę, aby uzyskać recenzje dla klasy produktów.
źródło
Nie umieściłbym funkcji „Uzyskaj recenzje dla produktu” ani w
Product
klasie, ani wReview
klasie ...Masz miejsce, w którym odzyskujesz swoje produkty, prawda? Coś z
GetProductById(int productId)
i możeGetProductsByCategory(int categoryId)
i tak dalej.Podobnie, powinieneś mieć miejsce, w którym możesz odzyskać swoje recenzje,
GetReviewbyId(int reviewId)
a może iGetReviewsForProduct(int productId)
.Jeśli oddzielisz dostęp do danych od klas domen, nie będziesz musiał zmieniać żadnej klasy domeny podczas zmiany sposobu zbierania recenzji.
źródło
Wzory i zasady są wytycznymi, a nie regułami zapisanymi w kamieniu. Moim zdaniem pytanie nie brzmi, czy lepiej jest przestrzegać zasad SOLID, czy zachować prostszy interfejs. Powinieneś zadać sobie pytanie, co jest bardziej czytelne i zrozumiałe dla większości ludzi. Często oznacza to, że musi być jak najbliżej domeny.
W takim przypadku wolałbym rozwiązanie (B), ponieważ dla mnie punktem wyjścia jest Produkt, a nie Recenzja, ale wyobraź sobie, że piszesz oprogramowanie do zarządzania recenzjami. W takim przypadku centrum stanowi przegląd, więc rozwiązanie (A) może być preferowane.
Kiedy mam wiele takich metod („połączenia” między klasami), usuwam je wszystkie na zewnątrz i tworzę jedną (lub więcej) nową klasę statyczną, aby je uporządkować. Zazwyczaj można je zobaczyć jako zapytania lub rodzaj repozytorium.
źródło
Twój produkt może po prostu delegować na metodę oceny statycznej, w którym to przypadku zapewniasz wygodny interfejs w naturalnej lokalizacji (Product.getReviews), ale szczegóły implementacji znajdują się w Review.getForProduct.
SOLID są wytycznymi i powinny dać prosty, rozsądny interfejs. Alternatywnie, możesz czerpać SOLID z prostych, rozsądnych interfejsów. Chodzi o zarządzanie zależnościami w kodzie. Celem jest zminimalizowanie zależności, które powodują tarcie i tworzą bariery dla nieuchronnych zmian.
źródło
Mam nieco inne podejście do tego niż większość innych odpowiedzi. Myślę, że
Product
iReview
są to w zasadzie obiekty do przesyłania danych (DTO). W moim kodzie staram się, aby moje DTO / podmioty unikały zachowań. Są po prostu dobrym API do przechowywania aktualnego stanu mojego modelu.Kiedy mówisz o OO i SOLID, ogólnie mówisz o „obiekcie”, który nie reprezentuje stanu (koniecznie), ale zamiast tego reprezentuje jakąś usługę, która odpowiada na pytania za ciebie, lub do której możesz przekazać część swojej pracy . Na przykład:
Wtedy Twój rzeczywisty
ProductRepository
zwróci wartośćExistingProduct
dlaGetProductByProductId
metody itp.Teraz postępujesz zgodnie z zasadą pojedynczej odpowiedzialności (wszystko, co dziedziczy po
IProduct
prostu trzyma się stanu, a wszystko, co dziedziczy,IProductRepository
jest odpowiedzialne za umiejętność zachowania i ponownego uwodnienia twojego modelu danych).Jeśli zmienisz schemat bazy danych, możesz zmienić implementację repozytorium bez zmiany DTO itp.
Krótko mówiąc, chyba nie wybrałbym żadnej z twoich opcji. :)
źródło
Metody statyczne mogą być trudniejsze do przetestowania, ale to nie znaczy, że nie możesz ich użyć - wystarczy je skonfigurować, aby nie trzeba było ich testować.
Utwórz zarówno product.GetReviews, jak i Review.ForProduct jednowierszowe metody, które są podobne
ReviewService zawiera bardziej złożony kod i ma interfejs zaprojektowany do testowania, ale nie jest udostępniany bezpośrednio użytkownikowi.
Jeśli musisz mieć 100% zasięgu, poproś, aby testy integracyjne wywołały metody klasy produktu / oceny.
Może to pomóc, jeśli myślisz o projekcie publicznego interfejsu API, a nie o projekcie klasy - w tym kontekście najważniejszy jest prosty interfejs z logicznym grupowaniem - faktyczna struktura kodu ma znaczenie tylko dla programistów.
źródło