Progettare una classe per prendere intere classi come parametri piuttosto che singole proprietà

Non c'è assolutamente nulla di sbagliato nel passare un intero oggetto User come parametro. In effetti, potrebbe aiutare a chiarire il tuo codice e rendere più ovvio ai programmatori l'utilizzo di un metodo se la firma del metodo richiede un User .

Passare semplici tipi di dati è bello, fino a quando non significano qualcosa di diverso da quello che sono. Considera questo esempio:

public class Foo
{
    public void Bar(int userId)
    {
        // ...
    }
}

E un esempio di utilizzo:

var user = blogPostRepository.Find(32);
var foo = new Foo();

foo.Bar(user.Id);

Puoi individuare il difetto? Il compilatore non può. L'ID utente che viene passato è solo un numero intero. Denominiamo la variabile user ma inizializziamo il suo valore dall'oggetto blogPostRepository , che presumibilmente restituisce BlogPost oggetti, non User oggetti - eppure il codice viene compilato e si finisce con un errore di runtime instabile.

Ora considera questo esempio modificato:

public class Foo
{
    public void Bar(User user)
    {
        // ...
    }
}

Forse il metodo Bar utilizza solo "ID utente" ma la firma del metodo richiede un oggetto User . Ora torniamo allo stesso esempio di utilizzo di prima, ma modificalo per passare l'intero "utente" in:

var user = blogPostRepository.Find(32);
var foo = new Foo();

foo.Bar(user);

Ora abbiamo un errore del compilatore. Il metodo blogPostRepository.Find restituisce un oggetto BlogPost , che chiamiamo abilmente "utente". Quindi passiamo questo "utente" al metodo Bar e otteniamo prontamente un errore del compilatore, perché non possiamo passare un BlogPost a un metodo che accetta User .

Il sistema di tipi del linguaggio viene sfruttato per scrivere codice corretto più veloce e identificare i difetti al momento della compilazione, piuttosto che il tempo di esecuzione.

In realtà, dovendo refactoring un sacco di codice perché le modifiche delle informazioni dell'utente è solo un sintomo di altri problemi. Superando un intero oggetto User ottieni i vantaggi di cui sopra, oltre ai vantaggi di non dover rifattorizzare tutte le firme dei metodi che accettano le informazioni dell'utente quando qualcosa sulla classe User cambia.

Am I right in thinking this is a violation of the Open/Closed principle?

No, non è una violazione di quel principio. Questo principio riguarda il non cambiare User in modi che influenzano altre parti del codice che lo usano. Le tue modifiche a User potrebbero essere tali violazioni, ma non correlate.

Are any other principles broken by this practice? Dependency inversion perahaps?

No. Quello che descrivi - solo iniettando le parti richieste di un oggetto utente in ogni metodo - è l'opposto: è pura inversione di dipendenza.

Are there other, non-SOLID principles being violated, such as basic defensive programming principles?

No. Questo approccio è un modo di codifica perfettamente valido. Non sta violando tali principi.

Ma l'inversione di dipendenza è solo un principio; non è una legge infrangibile. E la pura DI può aggiungere complessità al sistema. Se si scopre che solo iniettare i valori utente necessari in metodi, piuttosto che passare l'intero oggetto utente nel metodo o nel costruttore, crea problemi, quindi non farlo in questo modo. Si tratta di ottenere un equilibrio tra principi e pragmatismo.

Per indirizzare il tuo commento:

There are problems with having to unnecessarily parse a new value down five levels of the chain and then change all of the references to all five of those existing methods...

Parte del problema qui è che chiaramente non ti piace questo approccio, come per il commento "inutilmente [passa] ...". E questo è abbastanza giusto; non c'è una risposta giusta qui. Se lo trovi gravoso, non farlo in questo modo.

Tuttavia, per quanto riguarda il principio aperto / chiuso, se segui rigorosamente "... cambia tutti i riferimenti a tutti e cinque i metodi esistenti ..." è un'indicazione che quei metodi sono stati modificati, quando dovrebbero essere chiusi alla modifica. In realtà, tuttavia, il principio aperto / chiuso è utile per le API pubbliche, ma non ha molto senso per i componenti interni di un'app.

...but surely a plan to abide by that principle as far as it is practicable to do so would include strategies to reduce the need for future change?

Ma poi ti godi in territorio YAGNI e sarebbe comunque ortogonale al principio. Se hai metodo Foo che accetta un nome utente e poi vuoi Foo per prendere una data di nascita, seguendo il principio, aggiungi un nuovo metodo; Foo rimane invariato. Ancora una volta è una buona pratica per le API pubbliche, ma non ha senso per il codice interno.

Come accennato in precedenza, si tratta di equilibrio e buon senso per ogni situazione. Se questi parametri cambiano spesso, allora sì, usa User direttamente. Ti salverà dalle modifiche su larga scala che descrivi. Ma se non cambiano spesso, passare solo ciò che è necessario è un buon approccio.

Sì, la modifica di una funzione esistente è una violazione del Principio Aperto / Chiuso. Stai modificando qualcosa che dovrebbe essere chiuso alle modifiche a causa del cambiamento dei requisiti. Un design migliore (per non cambiare quando cambiano i requisiti) dovrebbe passare all'utente nelle cose che dovrebbe funzionare sugli utenti.

Ma questo potrebbe scappare dal principio di segregazione dell'interfaccia, dal momento che potresti passare più informazioni modo di quante la funzione abbia bisogno di fare il suo lavoro.

Quindi, come con la maggior parte delle cose - dipende .

Usando solo un nome utente, la funzione diventa più flessibile, lavorando con i nomi utente indipendentemente da dove provengono e senza la necessità di creare un oggetto Utente pienamente funzionante. Fornisce resilienza al cambiamento se si pensa che la fonte dei dati cambierà.

L'utilizzo dell'intero utente rende più chiaro l'utilizzo e stabilisce un contratto più stabile con i chiamanti. Fornisce resilienza al cambiamento se pensi che sarà necessario più Utente.

Questo design segue il Pattern degli oggetti dei parametri . Risolve i problemi derivanti dall'avere molti parametri nella firma del metodo.

Am I right in thinking this is a violation of the Open/Closed principle?

No. L'applicazione di questo modello abilita il principio Apri / chiudi (OCP). Ad esempio, le classi derivate di User possono essere fornite come parametro che inducono un comportamento diverso nella classe di consumo.

Are any other principles broken by this practice?

Può può accadere. Lasciatemi spiegare sulla base dei principi SOLIDI.

Il Principio di responsabilità singola (SRP) può essere violato se ha il design come hai spiegato:

This class exposes all information about the user, their Id, name, levels of access to each module, timezone etc.

Il problema è con tutte le informazioni . Se la classe User ha molte proprietà, diventa un enorme Oggetto di trasferimento dati che trasporta informazioni non correlate dal punto di vista del classi di consumo. Esempio: dal punto di vista di una classe di consumo UserAuthentication la proprietà User.Id e User.Name sono rilevanti, ma non User.Timezone .

Anche il principio di segregazione dell'interfaccia (ISP) viene violato con un ragionamento simile, ma aggiunge un'altra prospettiva. Esempio: supponiamo che una classe di consumo UserManagement richieda la proprietà User.Name da dividere fino a User.LastName e User.FirstName la classe UserAuthentication deve essere anch'essa modificata per questo.

Fortunatamente l'ISP offre anche una possibile via d'uscita dal problema: in genere tali oggetti parametrici o oggetti di trasporto dati iniziano in piccolo e crescono di volta in volta. Se questo diventa poco pratico, considera il seguente approccio: Introduci interfacce su misura per le esigenze delle classi consumatrici. Esempio: introdurre interfacce e lasciare che la classe User ne derivi:

class User : IUserAuthenticationInfo, IUserLocationInfo { ... }

Ogni interfaccia deve esporre un sottoinsieme di proprietà correlate della classe User necessaria per una classe che consuma per completare l'operazione. Cerca gruppi di proprietà. Prova a riutilizzare le interfacce. Nel caso della classe di consumo UserAuthentication usa IUserAuthenticationInfo invece di User . Quindi, se possibile, suddividi la classe User in più classi concrete usando le interfacce come "stencil".

Quando ho affrontato questo problema nel mio codice, ho concluso che le classi / gli oggetti del modello di base sono la risposta.

Un esempio comune sarebbe il pattern del repository. Spesso quando si esegue una query su un database tramite repository, molti dei metodi nel repository richiedono molti degli stessi parametri.

Le mie regole pratiche per i repository sono:

• Dove più di un metodo prende gli stessi 2 o più parametri, i parametri dovrebbero essere raggruppati come un oggetto modello.

• Quando un metodo richiede più di 2 parametri, i parametri dovrebbero essere raggruppati come un oggetto modello.

• I modelli possono ereditare da una base comune, ma solo quando ha un senso (di solito è meglio rifattorizzare più tardi che iniziare con l'ereditarietà in mente).

I problemi con l'utilizzo di modelli di altri livelli / aree non diventano evidenti finché il progetto non inizia a diventare un po 'complesso. È solo allora che trovi meno codice crea più lavoro o più complicazioni.

E sì, è assolutamente bene avere 2 diversi modelli con proprietà identiche che servono diversi livelli / scopi (ad esempio ViewModels vs POCOs).

Controlliamo solo i singoli aspetti di SOLID:

• Responsabilità singola: è possibile che si verifichi se le persone tendono a passare solo sezioni della classe.
• Apri / Chiuso: irrilevante dove vengono passate sezioni della classe, solo dove viene passato l'intero oggetto. (Penso che sia qui che entra in gioco la dissonanza cognitiva: devi cambiare un codice lontano ma la classe stessa sembra a posto.)
• Sostituzione di Liskov: non-problema, non stiamo facendo sottoclassi.
• inversione delle dipendenze (dipende dalle astrazioni, non dai dati concreti). Sì, è stato violato: le persone non hanno astrazioni, estrapolano elementi concreti della classe e le trasmettono. Penso che questo sia il problema principale qui.

Una cosa che tende a confondere gli istinti di progettazione è che la classe è essenzialmente per gli oggetti globali, ed essenzialmente di sola lettura. In una situazione del genere, violentare le astrazioni non fa molto male: solo la lettura di dati che non sono modificati crea un accoppiamento piuttosto debole; solo quando diventa una pila enorme il dolore diventa evidente.
Per ripristinare gli istinti del design, supponiamo che l'oggetto non sia molto globale. Di quale contesto sarebbe necessaria una funzione se l'oggetto User potesse essere mutato in qualsiasi momento? Quali componenti dell'oggetto verrebbero probabilmente mutati insieme? Questi possono essere suddivisi in User , sia come sottotitoli di riferimento sia come interfaccia che espone solo una "fetta" di campi correlati non è così importante.

Un altro principio: guarda le funzioni che usano parti di User e vedi quali campi (attributi) tendono ad andare insieme. Questa è una buona lista preliminare di sottooggetti: devi assolutamente pensare se effettivamente appartengono insieme.

È molto lavoro, ed è un po 'difficile da fare, e il tuo codice diventerà leggermente meno flessibile perché diventerà leggermente più difficile identificare il sub-oggetto (subinterface) che deve essere passato a una funzione, in particolare se i sottooggetti hanno sovrapposizioni.

Dividere User up in realtà diventerà brutto se i sottooggetti si sovrappongono, quindi le persone saranno confuse su quale scegliere se tutti i campi richiesti sono dalla sovrapposizione. Se ti dividi gerarchicamente (ad es. Hai UserMarketSegment che, tra le altre cose, ha UserLocation ), le persone non sono sicure su quale sia il livello della funzione in cui stanno scrivendo: si tratta di dati dell'utente al Location livello o al livello MarketSegment ? Non aiuta esattamente che questo possa cambiare nel tempo, cioè sei tornato a cambiare le firme delle funzioni, a volte attraverso un'intera catena di chiamate.

In altre parole: a meno che tu non conosca veramente il tuo dominio e abbia una chiara idea di quale modulo abbia a che fare con gli aspetti di User , non vale la pena migliorare la struttura del programma.

Questa è una domanda davvero interessante. Dipende.

Se pensi che il tuo metodo possa cambiare in futuro internamente per richiedere parametri diversi dell'oggetto User, dovresti sicuramente passare tutto. Il vantaggio è che il codice esterno al metodo viene quindi protetto dalle modifiche all'interno del metodo in termini di quali parametri sta utilizzando, cosa che, come dici tu, causerebbe una cascata di modifiche esternamente. Quindi passare l'intero utente aumenta l'incapsulamento.

Se sei abbastanza sicuro che non avrai mai bisogno di usare altro che dire l'e-mail dell'Utente, dovresti passarlo. Il vantaggio di questo è che puoi usare il metodo in una gamma più ampia di contesti: potresti usare ad esempio con l'e-mail di una società o con una e-mail che qualcuno ha appena digitato. Ciò aumenta la flessibilità.

Questo fa parte di una più ampia gamma di domande sulla creazione di classi per avere un ambito ampio o ristretto, includendo se iniettare o meno le dipendenze e se disporre di oggetti globalmente disponibili. C'è una sfortunata tendenza al momento di pensare che la portata più ristretta sia sempre buona. Tuttavia, c'è sempre un compromesso tra incapsulamento e flessibilità come in questo caso.

Trovo che sia meglio passare il minor numero possibile di parametri e il numero necessario di parametri. Ciò semplifica i test e non richiede la creazione di interi oggetti.

Nel tuo esempio, se userai solo l'user-id o il nome utente, questo è tutto ciò che dovresti passare. Se questo modello si ripete più volte e l'oggetto utente reale è molto più grande, il mio consiglio è di creare un'interfaccia più piccola per questo. Potrebbe essere

interface IIdentifieable
{
    Guid ID { get; }
}

o

interface INameable
{
    string Name { get; }
}

Questo rende molto più facile il testing con il mocking e tu sai immediatamente quali valori sono realmente usati. Altrimenti spesso è necessario inizializzare oggetti complessi con molte altre dipendenze, anche se alla fine hai solo bisogno di una o due proprietà.

Ecco qualcosa che ho incontrato di tanto in tanto:

• Un metodo accetta un argomento di tipo User (o Product o qualsiasi altra cosa) che ha molte proprietà, anche se il metodo ne usa solo alcune.
• Per qualche ragione, alcune parti del codice devono chiamare quel metodo anche se non ha un oggetto User completamente popolato. Crea un'istanza e inizializza solo le proprietà di cui il metodo ha effettivamente bisogno.
• Questo succede un sacco di volte.
• Dopo un po ', quando incontri un metodo che ha un argomento User , ti ritrovi a dover trovare chiamate a quel metodo per trovare da dove viene il User in modo da sapere quali proprietà sono popolate. È un utente "reale" con un indirizzo email o è stato appena creato per passare un ID utente e alcune autorizzazioni?

Se crei un User e popoli solo alcune proprietà perché quelle sono quelle necessarie al metodo, allora il chiamante sa molto di più sul funzionamento interno del metodo di quanto dovrebbe.

Peggio ancora, quando hai un'istanza di User , devi sapere da dove proviene in modo da sapere quali proprietà sono popolate. Non vuoi doverlo sapere.

Nel tempo, quando gli sviluppatori vedono User utilizzato come contenitore per gli argomenti del metodo, potrebbero iniziare ad aggiungere proprietà per gli scenari monouso. Ora sta diventando brutto, perché la classe è ingombra di proprietà che saranno quasi sempre nulle o predefinite.

Tale corruzione non è inevitabile, ma accade ripetutamente quando passiamo un oggetto solo perché abbiamo bisogno di accedere ad alcune delle sue proprietà. La zona di pericolo è la prima volta che vedi qualcuno che crea un'istanza di User e popola solo alcune proprietà in modo che possano passarlo a un metodo. Metti il piede su di esso perché è un sentiero oscuro.

Se possibile, imposta l'esempio giusto per il prossimo sviluppatore passando solo ciò che devi passare.