Metody SOLID vs. statyczne

11

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).

Wolfgang
źródło
2
Dopóki nie jest to zmienna statyczna, wszystko jest fajne. Metody statyczne proste do przetestowania i łatwe do prześledzenia.
Koder
3
Dlaczego nie mieć tylko klasy ProductsReviews? Następnie produkt i recenzja pozostają takie same. A może źle zrozumiałem.
ElGringoGrande
2
@Coder „Metody statyczne są łatwe do przetestowania”, naprawdę? Nie można ich wyśmiewać, zobacz: googletesting.blogspot.com/2008/12/... po więcej szczegółów.
StuperUser
1
@StuperUser: Nie ma co kpić. Assert(5 = Math.Abs(-5));
Koder
2
Testowanie 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 w Abs()(lub jego zależnym kodzie) i usuwa zalety diagnostyczne testów jednostkowych.
StuperUser

Odpowiedzi:

7

Co jest ważniejsze: przestrzeganie zasad SOLID lub prostszy interfejs?

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.

radarbob
źródło
1
+1 za zauważenie, że OCP jest raczej wskaźnikiem dobrej jakości, a nie szybką regułą dla dowolnego kodu. Jeśli okaże się, że prawidłowe lub zgodne z OCP jest trudne lub niemożliwe, oznacza to, że model musi zostać poddany refaktoryzacji, aby umożliwić bardziej elastyczne ponowne użycie.
CodexArcanum
Wybrałem przykład Produkt i Recenzja, aby wskazać, że Produkt jest istotą kluczową projektu, podczas gdy Recenzja jest jedynie dodatkiem. Produkt jest już gotowy, gotowy, a Recenzja pojawi się później i powinna zostać wprowadzona bez otwierania istniejącego kodu (w tym produktu).
Wolfgang,
10

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()to staticnie 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 w forProduct()(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ą.

StuperUser
źródło
1
To bardzo dobra uwaga. Ogólnie jednak nie dla mnie. ;-) Nie jestem zbyt surowy w pisaniu testów jednostkowych, które obejmują więcej niż jedną klasę. Wszyscy schodzą do bazy danych, to dla mnie w porządku. Nie będę więc kpił z obiektu biznesowego ani jego wyszukiwarki.
Wolfgang,
+1, dziękuję za ustawienie czerwonej flagi do testów! Zgadzam się z @Wolfgang, zwykle też nie jestem zbyt surowy, ale kiedy potrzebuję tego rodzaju testów, naprawdę nie znoszę metod statycznych. Zwykle, jeśli metoda statyczna zbyt mocno współdziała z jej parametrami lub jeśli wchodzi w interakcję z jakąkolwiek inną metodą statyczną, wolę uczynić ją metodą instancji.
Adriano Repetti,
Czy to nie zależy całkowicie od używanego języka? OP wykorzystał przykład Java, ale nigdy nie wspomniał o języku w pytaniu, ani nie został określony w tagach.
Izkata,
1
@StuperUser 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.
Izkata,
1
@Izkata JS jest dynamicznie wpisywany, więc nie ma go 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.
StuperUser
4

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 - TwitterReviewRetrieveralbo AmazonReviewRetrieveralbo MultipleSourceReviewRetrieveralbo 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 ProductWithReviewsgdyby naprawdę chciał być pedantyczny o swoich SOLID zasad, ale to byłoby wystarczająco dobre dla mnie).

Lunivore
źródło
Brzmi jak wzorzec DAO, który jest bardzo powszechny w oprogramowaniu zorientowanym na usługi / komponenty. Podoba mi się ten pomysł, ponieważ wyraża on, że pobieranie obiektów nie jest obowiązkiem tych obiektów. Ponieważ jednak wolę sposób zorientowany obiektowo niż zorientowany na usługi.
Wolfgang,
1
Użycie takiego interfejsu IRetrieveReviewssprawia, ż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.
Lunivore,
Tak, więc byłaby to implementacja wzorca strategii. Co byłoby argumentem za trzecią klasą. (A) i (B) nie poparłyby tego. Wyszukiwarka z pewnością użyje ORM, więc nie ma powodu, aby zastępować algorytm. Przepraszam, jeśli nie wyraziłem się jasno w moim pytaniu.
Wolfgang,
3

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.

ElGringoGrande
źródło
Nie zgadzam się, że zastosowanie metody Recenzji naruszyłoby SRP. Na produkcie, ale nie w recenzji. Mój problem polegał na tym, że jest statyczny. Gdybym przeniósł metodę do jakiejś trzeciej klasy, nadal byłby statyczny i miałby parametr produktu.
Wolfgang,
Mówisz więc, że jedyną odpowiedzialnością za klasę Review, jedynym powodem jej zmiany, jest konieczność zmiany metody statycznej forProduct? Nie ma innych funkcji w klasie Review?
ElGringoGrande
Recenzja zawiera wiele rzeczy. Ale myślę, że forProduct (produkt) dobrze do niego pasuje. Znajdowanie recenzji zależy w dużej mierze od atrybutów i struktury recenzji (które unikalne identyfikatory, który zakres zmienia atrybuty?). Produkt jednak nie wie i nie powinien wiedzieć o recenzjach.
Wolfgang,
3

Nie umieściłbym funkcji „Uzyskaj recenzje dla produktu” ani w Productklasie, ani w Reviewklasie ...

Masz miejsce, w którym odzyskujesz swoje produkty, prawda? Coś z GetProductById(int productId)i może GetProductsByCategory(int categoryId)i tak dalej.

Podobnie, powinieneś mieć miejsce, w którym możesz odzyskać swoje recenzje, GetReviewbyId(int reviewId)a może i GetReviewsForProduct(int productId).

Kiedy chcę zmienić sposób gromadzenia recenzji produktu, muszę zmienić klasę produktu.

Jeśli oddzielisz dostęp do danych od klas domen, nie będziesz musiał zmieniać żadnej klasy domeny podczas zmiany sposobu zbierania recenzji.

Eric King
źródło
Tak sobie z tym poradzę i jestem zdezorientowany (i martwię się o to, czy moje własne pomysły są właściwe) z powodu braku tej odpowiedzi do twojego postu. Oczywiście ani klasa reprezentująca raport, ani reprezentacja produktu nie powinny być odpowiedzialne za faktyczne odzyskanie drugiego. Niektóre rodzaje usług dostarczających dane powinny sobie z tym poradzić.
CodexArcanum
@CodexArcanum Cóż, nie jesteś sam. :-)
Eric King
2

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.

Adriano Repetti
źródło
Zastanawiałem się także nad tym pojęciem semantyki obu końców i który jest „silniejszy”, aby zajął się poszukiwaczem. Właśnie dlatego wymyśliłem (B). Pomogłoby to również stworzyć hierarchię „abstrakcji” klas biznesowych. Produkt był jedynie podstawowym przedmiotem, w którym Recenzja jest wyższa. Tak więc Recenzja może odnosić się do Produktu, ale nie na odwrót. W ten sposób można uniknąć cykli w referencjach między klasami biznesowymi, co stanowiłoby kolejny problem na mojej liście rozwiązany.
Wolfgang,
Myślę, że w przypadku niewielkiego zestawu klas fajnie byłoby podać OBU metod (A) i (B). Zbyt wiele razy interfejs użytkownika nie jest dobrze znany w chwili pisania tej logiki.
Adriano Repetti,
1

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.

pfries
źródło
Nie jestem pewien, czy bym tego chciał. W ten sposób mam metodę w obu miejscach, co jest dobre, ponieważ inni programiści nie muszą patrzeć w obu klasach, aby zobaczyć, gdzie ją umieściłem. Z drugiej strony miałbym zarówno wady metody statycznej, jak i zmianę „zamkniętej” klasy produktu. Nie jestem pewien, czy delegacja pomaga rozwiązać problem tarcia. Jeśli kiedykolwiek byłby powód do zmiany wyszukiwarki, z pewnością spowoduje to zmianę podpisu, a tym samym zmianę Produktu ponownie.
Wolfgang,
Kluczem OCP jest to, że możesz zastępować implementacje bez zmiany kodu. W praktyce jest to ściśle związane z substytucją Liskowa. Jedna implementacja musi być wymienna na inną. Możesz mieć DatabaseReviews i SoapReviews jako różne klasy, zarówno implementujące IGetReviews.getReviews, jak i getReviewsForProducts. Następnie twój system jest otwarty na rozszerzenie (zmiana sposobu otrzymywania recenzji) i zamknięty dla modyfikacji (zależności od IGetReviews nie psują się). To mam na myśli przez zarządzanie zależnościami. W twoim przypadku będziesz musiał zmodyfikować kod, aby zmienić jego zachowanie.
pfries
Czy nie jest to opisana przez ciebie zasada odwrócenia zależności (DIP)?
Wolfgang,
Są to powiązane zasady. Twój punkt zamiany jest osiągany przez DIP.
pfries
1

Mam nieco inne podejście do tego niż większość innych odpowiedzi. Myślę, że Producti Reviewsą 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:

interface IProductRepository
{
    void SaveNewProduct(IProduct product);
    IProduct GetProductById(ProductId productId);
    bool TryGetProductByName(string name, out IProduct product);
}

interface IProduct
{
    ProductId Id { get; }
    string Name { get; }
}

class ExistingProduct : IProduct
{
    public ProductId Id { get; private set; }
    public string Name { get; private set; }
}

Wtedy Twój rzeczywisty ProductRepositoryzwróci wartość ExistingProductdla GetProductByProductIdmetody itp.

Teraz postępujesz zgodnie z zasadą pojedynczej odpowiedzialności (wszystko, co dziedziczy po IProductprostu trzyma się stanu, a wszystko, co dziedziczy, IProductRepositoryjest 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. :)

Scott Whitlock
źródło
0

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

new ReviewService().GetReviews(productID);

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.

Tom Clarkson
źródło