In questo modo sto scrivendo questo codice è testabile, ma c'è qualcosa che non va mi manca?

Avere uno anziché molti parametri nel costruttore è non la parte problematica di questo design . Finché la tua classe IContext non è altro che una facciata del servizio , specificatamente per fornire le dipendenze utilizzate in MyControllerBase , e non un localizzatore di servizi generale utilizzato in tutto il tuo intero codice, quella parte del tuo codice è IMHO ok.

Il tuo primo esempio potrebbe essere cambiato in

protected MyControllerBase(IContext ctx)
{
    TheContext = ctx;
    SomeService = ctx.GetSomeService();
    AnotherService = ctx.GetAnotherService();
}

non sarebbe una modifica sostanziale del design di MyControllerBase . Se questo design è buono o cattivo dipende solo dal fatto che tu voglia

• assicurati che TheContext , SomeService e AnotherService siano sempre tutti inizializzati con oggetti mock, o tutti loro con oggetti reali
• o, per consentire l'inizializzazione con combinazioni diverse dei 3 oggetti (il che significa che in questo caso sarebbe necessario passare i parametri individualmente)

Quindi utilizzare solo un parametro anziché 3 nel costruttore può essere del tutto ragionevole.

La cosa che è problematica è IContext che espone il metodo GetService in pubblico. IMHO dovresti evitare questo, invece tieni espliciti i "metodi di fabbrica". Quindi sarà ok implementare i metodi GetSomeService e GetAnotherService dal mio esempio usando un localizzatore di servizi? IMHO che dipende. Finché la classe IContext continua a essere solo una semplice fabbrica astratta allo scopo specifico di fornire un elenco esplicito di oggetti di servizio, ciò è IMHO accettabile. Le fabbriche astratte sono in genere solo codice "colla", che non deve essere sottoposto a test dell'unità stessa. Nondimeno dovresti chiederti se nel contesto di metodi come GetSomeService , se hai davvero bisogno di un localizzatore di servizi, o se una chiamata al costruttore esplicita non sarebbe più semplice.

Quindi attenzione, quando ti limiti a un progetto in cui l'implementazione di IContext è solo un wrapper attorno a un metodo pubblico, generico GetService , che consente di risolvere qualsiasi depenibilità arbitraria da classi arbitrarie, quindi tutto si applica a ciò che @BenjaminHodgson ha scritto in la sua risposta.

Questo design è noto come Service Locator * e non mi piace. Ci sono molti argomenti contro di esso:

Service Locator ti accoppia al tuo contenitore . Usando l'iniezione di dipendenza regolare (in cui il costruttore esplicita esplicitamente le dipendenze) puoi semplicemente sostituire il tuo contenitore con uno diverso, o tornare a new -espressioni. Con il tuo IContext che non è proprio possibile.

L'individuazione dei servizi nasconde le dipendenze . Come cliente, è molto difficile dire cosa è necessario per costruire un'istanza di una classe. Hai bisogno di una sorta di IContext , ma tu anche devi impostare il contesto in alto per restituire gli oggetti corretti in modo che il MyControllerBase funzioni. Questo non è affatto ovvio dalla firma del costruttore. Con il DI regolare, il compilatore ti dice esattamente ciò di cui hai bisogno. Se la tua classe ha molte dipendenze, dovresti provare quel dolore perché ti spronerà al refactoring. Service Locator nasconde i problemi con cattivi design.

Service Locator causa errori di runtime . Se chiami GetService con un parametro di tipo errato otterrai un'eccezione. In altre parole, la tua funzione GetService non è una funzione totale. (Le funzioni totali sono un'idea del mondo FP, ma in pratica significa che le funzioni dovrebbero sempre restituire un valore.) È meglio lasciare che il compilatore ti aiuti e ti dica quando hai sbagliato le dipendenze.

Service Locator viola il Principio di sostituzione di Liskov . Poiché il suo comportamento varia in base all'argomento del tipo, è possibile visualizzare Service Locator come se avesse effettivamente un numero infinito di metodi sull'interfaccia! Questo argomento è spiegato dettagliatamente qui .

L'individuazione dei servizi è difficile da testare . Hai dato un esempio di una percentuale falsa di% co_de per i test, che va bene, ma sicuramente è meglio non dover scrivere quel codice in primo luogo. Basta iniettare direttamente le tue false dipendenze, senza passare per il tuo servizio di localizzazione.

In breve, solo non farlo . Sembra una soluzione seducente al problema delle classi con molte dipendenze, ma a lungo termine ti renderà la vita infelice.

_{* Sto definendo Service Locator come un oggetto con un metodo generico IContext che è in grado di risolvere dipendenze arbitrarie e viene utilizzato in tutto il codebase (non solo in Composition Root). Questo non è lo stesso di Service Facade (un oggetto che raggruppa alcune piccole serie conosciute di dipendenze) o Abstract Factory (un oggetto che crea istanze di un singolo tipo - il tipo di una Fabbrica astratta può essere generico ma il metodo non lo è).}

I migliori argomenti contro l'anti-pattern di Service Locator sono chiaramente indicati da Mark Seemann quindi non mi dilungherò troppo sul perché questa sia una cattiva idea - è un viaggio di apprendimento che devi prendere il tempo per capire da solo (ti consiglio anche Mark's book ).

OK per rispondere alla domanda - ripetiamo il tuo problema reale :

So rather than adding multiple parameters in the constructor for each service (because this will get really annoying and time-consuming for the developers extending the application) I am using this method to get services.

C'è una domanda che risolve questo problema su StackOverflow . Come menzionato in uno dei commenti qui:

The best remark: "One of the wonderful benefits of Constructor Injection is that it makes violations of the Single Responsibility Principle glaringly obvious."

Stai cercando nel posto sbagliato la soluzione al tuo problema. È importante sapere quando una classe sta facendo troppo. Nel tuo caso, sospetto strongmente che non sia necessario un "controller di base". Infatti, in OOP c'è quasi sempre nessuna necessità di ereditarietà . Variazioni di comportamento e funzionalità condivise possono essere raggiunte interamente attraverso l'uso appropriato di interfacce, che di solito si traduce in codice fattorizzato e incapsulato migliore e non è necessario passare dipendenze ai costruttori di superclasse.

In tutti i progetti su cui ho lavorato dove c'è un controller di base, è stato fatto esclusivamente allo scopo di condividere proprietà e metodi convenienti, come IsUserLoggedIn() e GetCurrentUserId() . STOP . Questo è un orribile abuso di eredità. Invece, crea un componente che espone questi metodi e prendi una dipendenza su di esso dove ti serve. In questo modo, i tuoi componenti rimarranno testabili e le loro dipendenze saranno evidenti.

A prescindere da qualsiasi altra cosa, quando si utilizza il pattern MVC consiglierei sempre skinny controller . Puoi leggere ulteriori informazioni su questo qui ma l'essenza del modello è semplice, che i controller in MVC dovrebbero fare solo una cosa: gestire gli argomenti passati dal framework MVC, delegando altri problemi ad altri componenti. Questo è di nuovo il Principio di Responsabilità Unica al lavoro.

Sarebbe davvero utile conoscere il tuo caso d'uso per ottenere un giudizio più accurato, ma onestamente non riesco a pensare a nessuno scenario in cui una classe base sia preferibile a dipendenze ben calcolate.

Sto aggiungendo una risposta a questo in base ai contributi di tutti gli altri. Grazie mille a tutti. Prima ecco la mia risposta: "No, non c'è niente di sbagliato in questo".

Risposta "Facciata di servizio" di Doc Brown Ho accettato questa risposta perché quello che stavo cercando (se la risposta era "no") era qualche esempio o espansione su quello che stavo facendo. Ha fornito questo nel suggerire che A) ha un nome e B) ci sono probabilmente modi migliori per farlo.

Risposta "Service Locator" di Benjamin Hodgson Per quanto apprezzi la conoscenza che ho acquisito qui, quello che ho non è un "Service Locator". È una "facciata di servizio". Tutto in questa risposta è corretto ma non per le mie circostanze.

Risposte della USR

Ne parlerò più in dettaglio:

You're giving up a lot of static information this way. You're deferring decisions to runtime like many dynamic languages do. That way you loose static verification (safety), documentation and tooling support (auto-complete, refactorings, find usages, data flow).

Non perdo gli strumenti e non sto perdendo alcuna digitazione "statica". La facciata del servizio restituirà ciò che ho configurato nel mio contenitore DI o default(T) . E ciò che restituisce è "digitato". Il riflesso è incapsulato.

I don't see why adding additional services as constructor arguments is a big burden.

Non è certamente "raro". Dato che sto usando un controller base , ogni volta che ho bisogno di cambiarlo, potrei dover cambiare 10, 100, 1000 altri controller.

If you use a dependency injection framework you will not even have to manually pass in the parameter values. Then again you loose some static advantages but not as many.

Sto usando un'iniezione di dipendenza. Questo è il punto.

E infine, il commento di Jules sulla risposta di Benjamin Non sto perdendo alcuna flessibilità. È la mia facciata del servizio . Posso aggiungere tanti parametri a GetService<T> in quanto voglio distinguere tra diverse implementazioni, proprio come si farebbe quando si configura un contenitore DI. Ad esempio, potrei cambiare GetService<T>() in GetService<T>(string extraInfo = null) per aggirare questo "potenziale problema".

Comunque, grazie ancora a tutti. È stato veramente utile. Cin cin.