SOLID rispetto ai metodi statici

11

Ecco un problema in cui mi imbatto frequentemente: ci sia un progetto di negozio web che abbia una classe di prodotto. Voglio aggiungere una funzione che consente agli utenti di pubblicare recensioni su un prodotto. Quindi ho una classe di revisione che fa riferimento a un prodotto. Ora ho bisogno di un metodo che elenchi tutte le recensioni su un prodotto. Ci sono due possibilità:

(A)

public class Product {
  ...
  public Collection<Review> getReviews() {...}
}

(B)

public class Review {
  ...
  static public Collection<Review> forProduct( Product product ) {...}
}

Guardando il codice, sceglierei (A): non è statico e non ha bisogno di un parametro. Tuttavia, ho la sensazione che (A) violi il Principio di Responsabilità Unica (SRP) e il Principio Open-Closed (OCP) mentre (B) no:

  • (SRP) Quando voglio cambiare il modo in cui le recensioni vengono raccolte per un prodotto, devo cambiare la classe del prodotto. Ma dovrebbe esserci una sola ragione per cambiare la classe del prodotto. E non sono certamente le recensioni. Se impacchetta tutte le funzionalità che hanno qualcosa a che fare con i prodotti nel Prodotto, presto verrà rumorosa.

  • (OCP) Devo cambiare la classe Product per estenderla con questa funzione. Penso che questo violi la parte "chiusa per il cambiamento" del principio. Prima di ottenere la richiesta del cliente per l'implementazione delle recensioni, ho considerato Prodotto come terminato e "chiuso".

Che cosa è più importante: seguire i principi SOLID o avere un'interfaccia più semplice?

O sto facendo qualcosa di sbagliato qui del tutto?

Risultato

Wow, grazie per tutte le tue fantastiche risposte! È difficile sceglierne una come risposta ufficiale.

Consentitemi di riassumere gli argomenti principali delle risposte:

  • pro (A): l'OCP non è una legge e anche la leggibilità del codice è importante.
  • pro (A): la relazione dell'entità dovrebbe essere navigabile. Entrambe le classi potrebbero conoscere questa relazione.
  • pro (A) + (B): esegui entrambi e delegati in (A) a (B) in modo che il Prodotto sia meno probabile che venga cambiato di nuovo.
  • pro (C): metti i metodi finder in terza classe (servizio) dove non è statico.
  • contra (B): impedisce il mocking nei test.

Alcune altre cose che i miei colleghi al lavoro hanno contribuito:

  • pro (B): il nostro framework ORM può generare automaticamente il codice per (B).
  • pro (A): per ragioni tecniche del nostro framework ORM, in alcuni casi sarà necessario cambiare l'entità "chiusa", indipendentemente da dove si trova il finder. Quindi non sarò sempre in grado di attenermi a SOLID, comunque.
  • contra (C): molto chiasso; -)

Conclusione

Sto usando entrambi (A) + (B) con delega per il mio progetto corrente. In un ambiente orientato ai servizi, tuttavia, andrò con (C).

    
posta Wolfgang 15.03.2012 - 16:16
fonte

9 risposte

7

What is more important: following the SOLID principles, or having a simpler interface?

Interfaccia vis-a-vis SOLIDA

Questi non si escludono a vicenda. L'interfaccia dovrebbe esprimere idealmente la natura del modello di business in termini di modello di business. I principi SOLID sono un Koan per massimizzare la manutenibilità del codice orientato agli oggetti (e intendo la "manutenibilità" nel senso più ampio). Il primo supporta l'uso e la manipolazione del modello di business e quest'ultimo ottimizza la manutenzione del codice.

Principio aperto / chiuso

"non toccarlo!" è un'interpretazione troppo semplicistica. E supponendo che intendiamo "la classe" è arbitraria e non necessariamente corretta. Piuttosto, OCP significa che hai progettato il tuo codice in modo tale che la modifica del suo comportamento non richiede (non dovrebbe) di modificare direttamente il codice esistente e funzionante. Inoltre, non toccare il codice in primo luogo è il modo ideale per preservare l'integrità delle interfacce esistenti; questo è un corollario significativo di OCP a mio avviso.

Finalmente vedo l'OCP come un indicatore della qualità del design esistente. Se mi trovo a scomporre classi aperte (o metodi) troppo spesso, e / o senza solide ragioni (ha, ha) per farlo, allora questo potrebbe dirmi che ho un pessimo design (e / o non lo faccio sapere come codificare OO).

Fai del tuo meglio, abbiamo un gruppo di medici in attesa

Se l'analisi dei requisiti ti dice che è necessario esprimere la relazione Product-Review da entrambe le prospettive, allora fallo.

Pertanto, Wolfgang, potresti avere una buona ragione per modificare quelle classi esistenti. Dati i nuovi requisiti, se una revisione è ora una parte fondamentale di un prodotto, se ogni estensione del prodotto ha bisogno di revisione, se così facendo rende il codice cliente espressivo in modo appropriato, quindi integrarlo nel prodotto.

    
risposta data 15.03.2012 - 22:16
fonte
11

SOLID sono linee guida, quindi influenza il processo decisionale piuttosto che dettarlo.

Una cosa da sapere quando si usano metodi statici è il loro impatto su testabilità .

Il test di forProduct(Product product) non costituirà un problema.

Provare qualcosa che dipende da ciò sarà

Non avrai una cucitura per isolare il Code-Under-Test (CUT) dipendente per usare una simulazione, poiché quando l'applicazione è in esecuzione, i metodi statici necessariamente esistono.

Abbiamo un metodo chiamato CUT() che chiama forProduct()

Se forProduct() è static non puoi testare CUT() come unità atomica e tutti i tuoi test diventano test di integrazione che testano la logica dell'unità.

Un errore in un test per CUT, potrebbe essere causato da un problema in CUT() o in forProduct() (o qualsiasi del suo codice dipendente) che rimuove i benefici di diagnosi dei test unitari.

Guarda questo eccellente post sul blog per informazioni più dettagliate: link

Questo può portare a frustrazione con test falliti e abbandono di buone pratiche e vantaggi che li circondano.

    
risposta data 15.03.2012 - 19:01
fonte
4

Se ritieni che il Prodotto sia il posto giusto per trovare le Recensioni del prodotto, puoi sempre dare al Prodotto una classe che aiuta a fare il lavoro per esso. (Puoi dirlo perché la tua azienda non parlerà mai di una recensione se non in termini di prodotto).

Ad esempio, sarei tentato di iniettare qualcosa che ha svolto il ruolo di un retriever di revisione. Probabilmente gli darei l'interfaccia IRetrieveReviews . È possibile inserire questo nel costruttore del prodotto (Dependency Injection). Se desideri modificare il modo in cui vengono recuperate le recensioni, puoi farlo facilmente inserendo un altro collaboratore: un TwitterReviewRetriever o un AmazonReviewRetriever o MultipleSourceReviewRetriever o qualsiasi altra cosa di cui hai bisogno.

Entrambe ora hanno una sola responsabilità (essendo l'elemento decisivo per tutto ciò che riguarda il prodotto e il recupero delle recensioni, rispettivamente), e in futuro il comportamento del prodotto rispetto alle recensioni può essere modificato senza effettivamente cambiare il prodotto (tu potrebbe estenderlo come ProductWithReviews se volessi essere pedante dei tuoi principi SOLID, ma questo sarebbe abbastanza buono per me).

    
risposta data 15.03.2012 - 18:07
fonte
3

Avrei una classe ProductsReview. Dici che la recensione è nuova comunque. Ciò non significa che possa essere qualsiasi cosa. Deve ancora avere una sola ragione per cambiare. Se cambi il modo in cui ottieni le recensioni per qualsiasi motivo, dovresti cambiare la classe di Revisione.

Questo non è corretto.

Stai mettendo il metodo statico nella classe Revisione perché ... perché? Non è quello con cui stai lottando? Non è questo l'intero problema?

Allora non farlo. Fai una lezione che è la sola responsabilità di ottenere le recensioni dei prodotti. Puoi quindi creare una sottoclasse in ProductReviewsByStartRating in qualsiasi modo. Oppure esegui la sottoclasse per ottenere recensioni per una classe di prodotti.

    
risposta data 15.03.2012 - 18:16
fonte
3

Non inserisco la funzionalità "Ottieni recensioni per prodotto" nella classe Product né nella classe Review ...

Hai un posto dove recuperi i tuoi prodotti, giusto? Qualcosa con GetProductById(int productId) e forse GetProductsByCategory(int categoryId) e così via.

Allo stesso modo, dovresti avere un posto dove recuperare le tue recensioni, con GetReviewbyId(int reviewId) e forse GetReviewsForProduct(int productId) .

When I want to change the way reviews are collected for a product, I have to change the Product class.

Se separi l'accesso ai dati dalle tue classi di dominio, non dovrai modificare nessuna classe di dominio quando cambi il modo in cui le recensioni vengono raccolte.

    
risposta data 15.03.2012 - 23:59
fonte
2

Modelli e principi sono linee guida, non regole scritte nella pietra. Nella mia opinione la domanda non è se è meglio seguire i principi SOLID o mantenere un'interfaccia più semplice. Quello che dovresti chiedere a te stesso è ciò che è più leggibile e comprensibile alla maggior parte delle persone. Spesso questo significa che deve essere il più vicino possibile al dominio.

In questo caso preferirei la soluzione (B) perché per me il punto di partenza è il prodotto, non il Review ma imagine stai scrivendo un software per gestire le recensioni. In tal caso il centro è la Revisione in cui la soluzione (A) può essere preferibile.

Quando ho molti metodi come questo ("connessioni" tra classi) li spoglio tutti all'esterno e creo una (o più) nuova classe statica per organizzarli. Di solito puoi vederli come query o tipo di repository.

    
risposta data 15.03.2012 - 17:33
fonte
1

Il tuo prodotto può semplicemente delegare al tuo metodo di revisione statico, nel qual caso stai fornendo un'interfaccia conveniente in una posizione naturale (Product.getReviews) ma i dettagli dell'implementazione sono in Review.getForProduct.

SOLID sono linee guida e dovrebbero risultare in un'interfaccia semplice e ragionevole. In alternativa, è possibile derivare SOLID da interfacce semplici e sensibili. È tutto basato sulla gestione delle dipendenze all'interno del codice. L'obiettivo è ridurre al minimo le dipendenze che creano attriti e creano ostacoli all'inevitabile cambiamento.

    
risposta data 15.03.2012 - 17:44
fonte
1

Ho un approccio leggermente diverso rispetto alla maggior parte delle altre risposte. Penso che Product e Review siano fondamentalmente oggetti di trasferimento dati (DTO). Nel mio codice cerco di far sì che i miei DTO / entità evitino comportamenti. Sono solo una bella API per memorizzare lo stato attuale del mio modello.

Quando parli di OO e SOLID, in genere parli di un "oggetto" che non rappresenta lo stato (necessariamente), ma rappresenta invece un tipo di servizio che risponde a domande per te o al quale puoi delegare del tuo lavoro. Ad esempio:

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; }
}

Quindi il tuo ProductRepository effettivo restituirà un ExistingProduct per il metodo GetProductByProductId , ecc.

Ora stai seguendo il principio della responsabilità unica (qualunque cosa eredita da IProduct è semplicemente in attesa di stato, e qualsiasi cosa eredita da IProductRepository è responsabile per sapere come mantenere e reidratare il tuo modello dati).

Se si modifica lo schema del database, è possibile modificare l'implementazione del repository senza modificare i DTO, ecc.

Quindi, in breve, credo che non sceglierei nessuna delle tue opzioni. :)

    
risposta data 17.03.2012 - 00:34
fonte
0

I metodi statici potrebbero essere più difficili da testare, ma ciò non significa che non puoi utilizzarli: devi solo impostarli in modo che non sia necessario testarli.

Crea sia product.GetReviews e Review.ForProduct metodi a una riga che sono qualcosa di simile

new ReviewService().GetReviews(productID);

ReviewService contiene tutto il codice più complesso e ha un'interfaccia progettata per la testabilità, ma non è esposta direttamente all'utente.

Se devi avere una copertura del 100%, i tuoi test di integrazione chiamano i metodi della classe prodotto / recensione.

Potrebbe essere d'aiuto se pensate alla progettazione dell'API pubblica piuttosto che alla progettazione della classe: in quel contesto una semplice interfaccia con il raggruppamento logico è tutto ciò che conta - la struttura del codice reale è importante solo per gli sviluppatori.

    
risposta data 16.03.2012 - 03:27
fonte

Leggi altre domande sui tag