Il design di questa classe viola il principio di responsabilità singola?

63

Oggi ho avuto una discussione con qualcuno.

Stavo spiegando i vantaggi di avere un modello di dominio ricco rispetto a un modello di dominio anemico. E ho dimostrato il mio punto con una classe semplice simile a quella:

public class Employee
{
    public Employee(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastname;
    }

    public string FirstName { get private set; }
    public string LastName { get; private set;}
    public int CountPaidDaysOffGranted { get; private set;}

    public void AddPaidDaysOffGranted(int numberOfdays)
    {
        // Do stuff
    }
}

Mentre difendeva il suo approccio al modello anemico, uno dei suoi argomenti era: "Sono un credente in SOLID . Stai violando il principio di responsabilità singola (SRP) poiché entrambi rappresenti dati e esegui la logica nel stessa classe. "

Ho trovato questa affermazione davvero sorprendente, poiché seguendo questo ragionamento, qualsiasi classe che ha una proprietà e un metodo viola l'SRP, e quindi l'OOP in generale non è SOLIDO e programmazione funzionale è l'unica via per il paradiso.

Ho deciso di non rispondere ai suoi numerosi argomenti, ma sono curioso di sapere cosa pensa la comunità su questa domanda.

Se avessi risposto, avrei iniziato puntando al paradosso sopra menzionato, e poi indicherei che l'SRP è strongmente dipendente dal livello di granularità che vuoi considerare e che se lo prendi abbastanza lontano, qualsiasi classe che contiene più di una proprietà o un metodo lo violano.

Che cosa avresti detto?

Aggiornamento: l'esempio è stato ampiamente aggiornato da guntbert per rendere il metodo più realistico e ci aiuta a concentrarci sulla discussione sottostante.

    
posta tobiak777 07.01.2016 - 22:35
fonte

11 risposte

68

La singola responsabilità dovrebbe essere intesa come un'astrazione di compiti logici nel proprio sistema. Una classe dovrebbe avere la sola responsabilità di (fare tutto il necessario per) eseguire un singolo compito specifico. Questo può effettivamente portare molto in una classe ben progettata, a seconda di quale sia la responsabilità. La classe che esegue il tuo motore di script, ad esempio, può avere un sacco di metodi e dati coinvolti nell'elaborazione degli script.

Il tuo collega si sta concentrando sulla cosa sbagliata. La domanda non è "quali membri hanno questa classe?" ma "quali operazioni utili svolge questa classe all'interno del programma?" Una volta capito, il tuo modello di dominio sembra perfetto.

    
risposta data 07.01.2016 - 22:41
fonte
41

Il principio di responsabilità singola riguarda solo se una parte di codice (in OOP, tipicamente stiamo parlando di classi) ha la responsabilità su un pezzo di funzionalità . Penso che il tuo amico stia dicendo che funzioni e dati non possono combinare una conversazione in realtà. Se Employee dovesse contenere anche informazioni sul suo posto di lavoro, quanto velocemente la sua macchina va, e che tipo di cibo il suo cane mangia allora avremmo un problema.

Poiché questa classe si occupa solo di Employee , penso che sia giusto dire che non viola in modo esplicito SRP, ma le persone hanno sempre le proprie opinioni.

Un luogo in cui potremmo migliorare è quello di separare le informazioni dei dipendenti (come nome, numero di telefono, e-mail) dalle sue vacanze. Nella mia mente, questo non significa che metodi e dati non possano combinare , significa solo che forse la funzionalità per le vacanze potrebbe essere in un luogo separato.

    
risposta data 07.01.2016 - 22:38
fonte
20

A mio parere, questa classe potrebbe potenzialmente violare SRP se continua a rappresentare sia Employee che EmployeeHolidays .

Così com'è, e se mi è venuta in mente per la Peer Review, probabilmente l'avrei lasciato passare. Se sono state aggiunte più proprietà e metodi specifici del dipendente e sono state aggiunte più proprietà specifiche per le festività, suggerirei probabilmente una suddivisione, citando sia SRP che ISP.

    
risposta data 07.01.2016 - 23:06
fonte
20

Ci sono già grandi risposte che indicano che SRP è un concetto astratto di funzionalità logica, ma ci sono punti più sottili che penso valga la pena aggiungere.

Le prime due lettere in SOLID, SRP e OCP riguardano sia il modo in cui il codice cambia in risposta a una modifica dei requisiti. La mia definizione preferita di SRP è: "un modulo / classe / funzione dovrebbe avere solo una ragione per cambiare". Discutere delle probabili ragioni per cui il codice deve cambiare è molto più produttivo che discutere se il tuo codice sia SOLIDO o no.

Quante ragioni deve cambiare la classe dei tuoi dipendenti? Non lo so, perché non conosco il contesto in cui lo stai usando e non riesco a vedere il futuro. Quello che posso fare è un brainstorming di possibili cambiamenti basati su ciò che ho visto in passato, e puoi valutare soggettivamente quanto sono probabili. Se più di un punteggio tra "ragionevolmente probabile" e "il mio codice è già cambiato per quella ragione esatta", allora stai violando SRP contro quel tipo di cambiamento. Eccone una: qualcuno con più di due nomi si unisce alla tua azienda (o un architetto legge questo eccellente articolo del W3C ). Eccone un altro: la tua azienda cambia il modo in cui assegna le festività.

Si noti che questi motivi sono ugualmente validi anche se si rimuove il metodo AddHolidays. Un sacco di modelli di dominio anemici violano l'SRP. Molti di questi sono solo database-code-in-code, ed è molto comune per le tabelle del database avere 20+ motivi per cambiare.

Ecco qualcosa da masticare: cambierebbe la tua classe dei Dipendenti se il tuo sistema dovesse tenere traccia dei salari dei dipendenti? Indirizzi? Informazioni di contatto di emergenza? Se hai detto "sì" (e "probabile che accada") a due di questi, la tua classe violerebbe SRP anche se non aveva ancora il codice! SOLID riguarda i processi e l'architettura tanto quanto il codice.

    
risposta data 08.01.2016 - 04:04
fonte
9

Che la classe rappresenti i dati non è responsabilità della classe, è un dettaglio di implementazione privato.

La classe ha una responsabilità, quella di rappresentare un dipendente. In questo contesto, ciò significa che presenta alcune API pubbliche che ti forniscono le funzionalità necessarie per trattare con i dipendenti (se AddHolidays è un buon esempio è discutibile).

L'implementazione è interna; succede così che questo abbia bisogno di alcune variabili private e qualche logica. Ciò non significa che la classe ora abbia più responsabilità.

    
risposta data 08.01.2016 - 09:47
fonte
5

L'idea che mescolare la logica e i dati in qualsiasi modo sia sempre sbagliata, è così ridicola che non merita nemmeno la discussione. Tuttavia, vi è in effetti una chiara violazione della singola responsabilità nell'esempio, ma non è perché c'è una proprietà DaysOfHolidays e una funzione AddHolidays(int) .

È perché l'identità del dipendente è mescolata con la gestione delle vacanze, il che è male. L'identità del dipendente è una cosa complessa che è necessaria per tenere traccia delle ferie, dello stipendio, degli straordinari, per indicare chi è su quale squadra, per collegare i rapporti sul rendimento, ecc. Un dipendente può anche cambiare nome, cognome o entrambi e rimanere lo stesso dipendente. I dipendenti possono anche avere più ortografie del loro nome, ad esempio un ASCII e un'ortografia Unicode. Le persone possono avere 0 o n primi e / o cognomi. Possono avere nomi diversi in diverse giurisdizioni. Tenere traccia dell'identità di un dipendente è sufficiente una responsabilità che la gestione di ferie o vacanze non può essere aggiunta in cima senza chiamarla una seconda responsabilità.

    
risposta data 09.01.2016 - 15:14
fonte
3

"I am a believer in SOLID. You are violating the single responsibility principle (SRP) as you are both representing data and performing logic in the same class."

Come gli altri, non sono d'accordo con questo.

Direi che l'SRP è violato se stai eseguendo più di un pezzo di logica nella classe. Quanti dati devono essere memorizzati all'interno della classe per ottenere quel singolo pezzo di logica è irrilevante.

    
risposta data 08.01.2016 - 02:41
fonte
2

In questi giorni non mi sembra utile discutere di ciò che fa e non costituisce una singola responsabilità o un singolo motivo per cambiare. Proporrei un principio minimo di dolore al suo posto:

Minimum Grief Principle: code should either seek to minimize its probability of requiring changes or maximize the ease of being changed.

Come va? Non dovresti prendere uno scienziato missilistico per capire perché questo può aiutare a ridurre i costi di manutenzione e, si spera, non dovrebbe essere un punto di discussione senza fine, ma come con SOLID in generale, non è qualcosa da applicare ciecamente ovunque. È qualcosa da considerare mentre si bilanciano i trade-off.

Per quanto riguarda la probabilità di richiedere modifiche, questo va giù con:

  1. Buona prova (affidabilità migliorata).
  2. Coinvolgere solo il codice minimo richiesto per fare qualcosa di specifico (questo può includere la riduzione degli accoppiamenti afferenti).
  3. Semplicemente facendo in modo che il codice funzioni in modo soddisfacente (vedi a fare il principio Badass).

Per quanto riguarda la difficoltà di apportare modifiche, va incontro a accoppiamenti efferenti. I test introducono accoppiamenti efferenti ma migliorano l'affidabilità. Fatto bene, generalmente fa più bene del male ed è totalmente accettabile e promosso dal Principio del lutto minimo.

Make Badass Principle: classes that are used in many places should be awesome. They should be reliable, efficient if that ties to their quality, etc.

E il principio Make Badass è legato al principio del dolore minimo, poiché le cose più difficili troveranno una probabilità inferiore di richiedere modifiche rispetto alle cose che fanno schifo in quello che fanno.

I would have started by pointing to the paradox mentioned above, and then indicate that the SRP is highly dependent on the level of granularity you want to consider and that if you take it far enough, any class containing more than one property or one method violates it.

Da un punto di vista della SRP una classe che a malapena fa qualsiasi cosa avrebbe certamente solo una (a volte zero) ragioni per cambiare:

class Float
{
public:
    explicit Float(float val);
    float get() const;
    void set(float new_val);
};

Questo praticamente non ha motivi per cambiare! È meglio di SRP. È ZRP!

Tranne che suggerirei che è in palese violazione del principio Make Badass. È anche assolutamente inutile. Qualcosa che fa così poco non può sperare di essere tosta. Ha troppe poche informazioni (TLI). E naturalmente quando hai qualcosa che è TLI, non può fare nulla di veramente significativo, nemmeno con le informazioni che incapsula, quindi deve farlo trapelare al mondo esterno nella speranza che qualcun altro possa effettivamente fare qualcosa di significativo e di cazzuto. E quel leakiness è ok per qualcosa che è solo inteso per aggregare dati e nient'altro, ma quella soglia è la differenza che vedo tra "data" e "objects".

Ovviamente qualcosa che è TMI è anche male. Potremmo mettere il nostro intero software in una classe. Può anche avere solo un metodo run . E qualcuno potrebbe anche obiettare che ora ha una ragione molto ampia per cambiare: "Questo corso dovrà essere modificato solo se il software ha bisogno di miglioramenti". Sono sciocco, ma naturalmente possiamo immaginare tutti i problemi di manutenzione con questo.

Quindi c'è un atto di bilanciamento per quanto riguarda la granularità o la ruvidezza degli oggetti che disegni. Spesso la giudico in base a quanta informazione devi svelare al mondo esterno e a quanta funzionalità significativa possa offrire. Trovo spesso utile il principio Make Badass per trovare l'equilibrio combinandolo con il Principio del lutto minimo.

    
risposta data 08.01.2016 - 00:50
fonte
1

Al contrario, per me il modello di dominio anemico rompe alcuni concetti principali di OOP (che legano gli attributi e il comportamento), ma può essere inevitabile sulla base di scelte architettoniche. I domini anemici sono più facili da pensare, meno organici e più sequenziali.

Molti sistemi tendono a farlo quando più livelli devono giocare con gli stessi dati (livello di servizio, livello web, livello client, agenti ...).

È più semplice definire la struttura dei dati in un posto e il comportamento in altre classi di servizio. Se la stessa classe è stata utilizzata su più livelli, questa può diventare grande e pone la domanda su quale layer è responsabile per definire il comportamento di cui ha bisogno e chi è in grado di chiamare i metodi.

Ad esempio, potrebbe non essere una buona idea di un processo di agente che calcola le statistiche su tutti i dipendenti che possono chiamare un calcolo per i giorni retribuiti. E la GUI dell'elenco dei dipendenti non ha certamente bisogno di tutto il nuovo calcolo dell'id aggregato usato in questo agente statistico (e dei dati tecnici che lo accompagnano). Quando si separano i metodi in questo modo, si termina generalmente con una classe con solo strutture di dati.

È possibile serializzare / deserializzare troppo facilmente i dati "oggetto", o solo alcuni di essi, o in un altro formato (json) ... senza preoccuparsi di alcun concetto / responsabilità dell'oggetto. È solo il passaggio di dati però. Puoi sempre fare il mapping dei dati tra due classi (Employee, EmployeeVO, EmployeeStatistic ...) ma cosa vuol dire Employee qui?

Quindi sì separa completamente i dati nelle classi di dominio e nella gestione dei dati nelle classi di servizio, ma è qui necessario. Tale sistema è allo stesso tempo funzionale a portare valore di business e anche tecnico per diffondere i dati ovunque sia necessario, pur mantenendo un ambito di responsabilità adeguato (e l'oggetto distribuito non risolve neanche questo).

Se non hai bisogno di separare gli ambiti di comportamento, sei più libero di mettere i metodi nelle classi di servizio o nelle classi di dominio, a seconda di come vedi il tuo oggetto. Tendo a vedere un oggetto come il concetto "reale", questo naturalmente aiuta a mantenere SRP. Quindi, nel tuo esempio, è più realistico che il Boss del dipendente aggiunga il giorno di paga concesso al suo Conto PayDay. Un dipendente è assunto dalla società, Works, può essere malato o essere invitato per un consiglio, e ha un account Payday (il Boss può recuperarlo direttamente da lui o da un registro PayDayAccount ...) Ma puoi creare una scorciatoia aggregata qui per semplicità se non vuoi troppa complessità per un semplice software.

    
risposta data 13.01.2016 - 23:35
fonte
0

Non puoi violare il principio di responsabilità singola perché è solo un criterio estetico, non una regola della natura. Non lasciarti ingannare dal nome dal suono scientifico e dalle lettere maiuscole.

    
risposta data 25.01.2016 - 18:13
fonte
0

You are violating the single responsibility principle (SRP) as you are both representing data and performing logic in the same class.

Sembra molto ragionevole per me. Il modello potrebbe non avere proprietà pubbliche se espone azioni. È fondamentalmente un'idea di separazione della query di comando. Tieni presente che il comando avrà uno stato privato di sicuro.

    
risposta data 25.01.2016 - 13:55
fonte

Leggi altre domande sui tag