Metody SOLID vs. statyczne

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

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.

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

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

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.

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.

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.

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.

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

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.