Laden...

Dependency Injection - Constructor Overload

Letzter Beitrag vor einem Jahr 16 Posts 1.028 Views
Dependency Injection - Constructor Overload

Moin,

ich will eine Wrapper Library für die Azure DevOps Api schreiben.
Und für mich und/oder andere Entwickler, soll die Anwendung der Library so einfach wie möglich gemacht werden.
Dafür war meine Überlegung, dass ich eine "AzDevOpsClient" Klasse habe, die instanziiert werden muss und man damit jeden Endpunkt ansprechen kann.

Für jeden verschiedenen Endpunkt der Api, dachte ich an jeweils einen Service.
Und die Services die ich dadurch erhalte, stelle ich in dem Client als Property zur Verfügung, somit diese bei notwendigkeit genutzt werden können, ohne den Service selber zu instanziieren oder irgendwie zu erhalten. Das ganze soll mit Lazy<T> umgesetzt werden, damit die Services erst erstellt werden, sobald der Service gebraucht wird.

Das ist derzeit mein 1. Entwurf von einem Service:

public class AccountService : IAccountService
{
   #region Fields
   private readonly IAccountApi _accountApi;
   #endregion //Fields
   public AccountService(HttpClient httpClient, ILogger<AccountService>? logger = null)
   {
       this._accountApi = RestService.For<IAccountApi>(this._httpClient);
   }
   public async Task<string> GetAllAsync(string? memberId, string? ownerId, string apiVersion = "7.0")//Implementation ignorieren
   {
       return await this._accountApi.GetAllAsync(memberId, ownerId, apiVersion);
   }
}

Und das ist mein 1. Entwurf der AzDevOpsClient Klasse:

public sealed class AzDevOpsClient
{
   #region Fields
   private Lazy<IAccountService> _account;
   private readonly IHttpClientFactory _httpClientFactory;
   private readonly ILoggerFactory? _loggerFactory;
   #endregion //Fields
   #region Properties
   public IAccountService Account => this._account?.Value ??
       (this._account = new(new AccountService(this._httpClientFactory.CreateClient(),
           this._loggerFactory?.CreateLogger<AccountService>()))).Value;
   #endregion //Properties
   public AzDevOpsClient(IHttpClientFactory httpClientFactory, ILoggerFactory? loggerFactory = null)
   {
       this._httpClientFactory = httpClientFactory ?? throw new ArgumentNullException(nameof(httpClientFactory));
       this._loggerFactory = loggerFactory;
       //AuthenticationService
   }
}

In der Client Klasse instanziiere ich derzeit noch hart den AccountService. Das wird noch geändert, sobald ich weiß, wie ich DI unterstützen kann.

Am Ende existieren bestimmt 30+ Services. Aber alle Interfaces der Services via den Konstruktor zu injecten, ist nicht zielführend. Wie handhabt man das am besten?

Ist das überhaupt ein guter Weg, dass so als Library anzubieten? Wär es vllt. besser, nur die einzelnen Services bereitzustellen ohne den generellen Client?
Ich als Entwickler, fänd das mit dem Client selbsterklärend, weil ich nicht Wissen muss (bzw. nach schauen), welche Services existieren, sondern auf die Properties zugreifen kann.

Das ganze soll nachher mit DI und ohne funktionieren können.

René

Ist das überhaupt ein guter Weg, dass so als Library anzubieten?

Prinzipiell ja; für jeden Endpunkt eine eigenen Service anbieten nicht. Service wäre hier auch der falsche Begriff. Es wäre ein Provider.
Services bilden i.d.R Logik ab - Provider macht externe Schnittstellen zugänglich.

Wir machen das generell auch immer so, wobei der Client (das Refit Interface) die gesamte API abbildet, und der Provider je nach Aufgabe getrennt wird.

public class AzureDevOpsUserAccountsProvider(IAzureDevOpsApiClient client)
public class AzureDevOpsPipelineProvider(IAzureDevOpsApiClient client)
public class AzureDevOpsIssuesProvider(IAzureDevOpsApiClient client)
public class AzureDevOpsReleasesProvider(IAzureDevOpsApiClient client)

PS: Da Du Refit verwendest, nutz doch auch die DI-Empfehlungen und -Möglichkeiten von Refit.

Wo Du keine DI hast, verwende den Factory Pattern (so wie der HTTP Client dafür auch einen Factory Pattern hat).

Zitat von Abt

Ist das überhaupt ein guter Weg, dass so als Library anzubieten?

Prinzipiell ja; für jeden Endpunkt eine eigenen Service anbieten nicht.

Ging auch nicht um jeden einzelnen Endpunkt, sondern wie du es ungefähr geschrieben hast.
Accounts, Pipelines, Build, Git, Audit..

Werde die "Services" auch zu Provider umbenennen. 
Aber nichtsdestotrotz werden viele Provder zusammen kommen. Und die alle im Konstruktor von dem Client aufzulisten, wär auch zu viel.
Wie könnte man das denn realisieren?

Aber nichtsdestotrotz werden viele Provder zusammen kommen. Und die alle im Konstruktor von dem Client aufzulisten, wär auch zu viel.

Der Provider kennt den Client, nicht umgekehrt.

Wir haben nicht einen einzigen Fall, dass über DI ettliche Provider übergeben werden müssen. Wir haben insgesamt IIRC nur 8 AZD Provider.
Warum ist das bei Dir so?

Wir verwenden ohnehin jedoch CQ(R)S über Handler. Wir haben also immer sehr schlanke Logikbausteine.
Service-Strukturen mit hunderten Methoden haben wir/haben moderne Software Architekturen genau wegen solchen Problemen nicht mehr.

Zitat von Abt

Aber nichtsdestotrotz werden viele Provder zusammen kommen. Und die alle im Konstruktor von dem Client aufzulisten, wär auch zu viel.

Der Provider kennt den Client, nicht umgekehrt.

Kannst du mir dafür ein Beispiel geben? Ich habe gerade keine Idee wie das umgesetzt werden sollte.

Warum das bei mir so ist? Ist einfach nach Aufgaben getrennt, dass jeder Provider bestimmte Endpunkte abdeckt.

Kannst du mir dafür ein Beispiel geben? Ich habe gerade keine Idee wie das umgesetzt werden sollte.

Steht doch oben

public class AzureDevOpsUserAccountsProvider(IAzureDevOpsApiClient client)

Oder was ist Deine Frage.

Du sagst, dass der Provider den Client kennt.

Aber was bringt mir das? Sobald man den Client nutzt, wie wird denn dann mit den Providern interagiert?

Oder soll der Entwickler sich die Provider holen anstatt den Client? Das war ja meine Eingangsfrage.

Oder soll der Entwickler sich die Provider holen anstatt den Client? Das war ja meine Eingangsfrage.

Ja.

Ein Client ist nur dazu dar die externe Schnittstelle zu definieren (Dein Refit Interface).
Ein Provider ist die Abstraktion der Nutzung. Der Provider macht zB das gesamte Client Handling ( Request, Exception...) sonst müsstest Du das ja jedes mal von Hand machen → DRY.

Deine Logik nutzt die Provider, nicht den Client.

Zitat von Abt

Ist das überhaupt ein guter Weg, dass so als Library anzubieten?

Prinzipiell ja; für jeden Endpunkt eine eigenen Service anbieten nicht. Service wäre hier auch der falsche Begriff. Es wäre ein Provider.
Services bilden i.d.R Logik ab - Provider macht externe Schnittstellen zugänglich.

Wir machen das generell auch immer so, wobei der Client (das Refit Interface) die gesamte API abbildet, und der Provider je nach Aufgabe getrennt wird.

Dann verstehe ich dein "Prinzipiell ja" nicht. Habe ich dich missverstanden?

Meine Frage war ja, einen Client zu haben, der die "Provider" als Properties anbietet um nicht jeden Provider manuell holen zu müssen.

Also meine Eingangsidee war:

IAzDevOpsClient client = GetClientFromAnyWhere();

//Methoden sind nur Beispiele
client.Accounts.GetAllAsync();
client.Builds.GetLogsAsync();
client.Audit.DeleteAllAsync();

Anstatt sowas machen zu müssen:

IAccountProvider accountProvider = GetProviderFromAnyWhere();
accountProvider.GetAllAsync();

IBuildProvider buildProvider = GetProviderFromAnyWhere();
buildProvider.GetLogsAsync();

Meine Frage war ja, einen Client zu haben, der die "Provider" als Properties anbietet um nicht jeden Provider manuell holen zu müssen.

Dann reden wir nicht von einem Client im eigentlichen Sinne. Denn den Client (das Refit Interface), den Du hier hast, kann keine Provider kennen - ergo keine Properties anbieten.

um nicht jeden Provider manuell holen zu müssen

Das Du "so viele Provider manuell holen musst" ist für mich ein Anzeichen, dass die Struktur, die "so viele Provider" erfordert, schon unglücklich ist - und die Wurzel der Ursache.

Flow sollte sein:

  • Client (Dein Refit Interface)
  • Provider bekommt Dein Client injiziert
  • Deine Logik bekommt den/die Provider injiziert

Wenn Du Dir ein Konstrukt baust mit

public class AzureDevOpsClient
{
    public AzureDevOpsUserAccountsProvider AzureDevOpsUserAccountsProvider {get;}

    public AzureDevOpsClient(refitClient)
    {
        AzureDevOpsUserAccountsProvider = new(refitClient)
    }

baust Du Dir etwas, mit dem Du die DI vereinfachst; gleichzeitig hast eben

  • etwas sehr schlecht/umständlich testbares (nur umsetzbar mit mehreren Factories)
  • verschleierst Du (glaube ich) das eigentliche Problem, dass Deine Logik "so viele Provider" gleichzeitig brauchst

Ich bin da kein Freund davon (was nichts heissen muss).
Vielleicht bin ich da auch gebrandmarkt vom Microsoft Graph Client: ieht exakt so aus und ist die mit Abstand schlimmste und qualitativ schlechteste Client-Implementierung, die mir je unter gekommen ist.

Zitat von Abt

Flow sollte sein:

  • Client (Dein Refit Interface)
  • Provider bekommt Dein Client injiziert
  • Deine Logik bekommt den/die Provider injiziert

Das ist genau das was ich meine.

Nur meine ich mit "Client", nicht den Refit Client (Interface, oder wie man es auch nennen mag).

"Deine Logik bekommt den/die Provider injiziert", meine ich mit Client.

Und dieser "Client" hat als Properties die Provider, die von außen aufgerufen werden können.

Nur bin ich wieder an dem Punkt, an dem ich nicht weiß, wie ich die Provider injizieren soll, wenn es z.B. 10 Stück sind, ohne den Konstruktor zu überladen.

Vielleicht bin ich da auch gebrandmarkt vom Microsoft Graph Client: ieht exakt so aus und ist die mit Abstand schlimmste und qualitativ schlechteste Client-Implementierung, die mir je unter gekommen ist.

So ähnlich hatte ich mir das vorgestellt, weil ich persönlich die Struktur gut finde. Darf ich Fragen, was du daran so schlimm findest? Und ggfs. andere Beispiele hast für so einen "Client" der deiner Meinung nach, eine gute Struktur besitzt?

Zitat von Duesmannr

Darf ich Fragen, was du daran so schlimm findest?

Schaust Du Dir den Graph Aufbau an, so siehst Du, dass dieser riesig ist.
Ich bin also gezwungen Zeug zu instanziieren, das ich für meine Verwendung einer einzelnen Operation nicht benötige. Der Overhead ist riesig.

Dazu noch fürchterlicher, automatisch generierter Code ohne jegliche Möglichkeit von Generics, was zu ständigen Code-Wiederholungen führt. Könnte darüber Bücher schreiben, wie beschissen das Ding umgesetzt ist. Allein die Klasse BaseRequestBuilder sowie dessen Verwendung in ganzen Provider Requests Implementierungen ist eine absolute Zumutung.

Reaktion von Microsoft zu Community-Feedback dazu: "Wir haben eher Feedback erhalten und nehmen es in die weitere Planung auf."
Was das bedeutet, ist auch klar... es wird sich nichts ändern 😃

Und ggfs. andere Beispiele hast für so einen "Client" der deiner Meinung nach, eine gute Struktur besitzt?

Kann ich Dir nun aus dem Stehgreif nichts sagen. In den meisten Fällen vermeide ich jede Art von solchen überladenen "Clients", wie Du es vor hast und implementier es lieber selbst mit Refit: schlank und so wie ich das brauche.
Bin ich schneller, effizienter und hab ich keine Update-Probleme (wie sie ständig mit dem Graph SDK und Breaking Changes der Fall sind). Abstrahiert werden muss das Zeug ohnehin, denn die SDKs von Microsoft liefern i.d.R. keine Interfaces für eine Abstraktion der Sache. Das heisst, dass der Aufwand ohnehin dazu kommt.

Wenn ich seh, was da die Azure DevOps API alles bietet. "Ich" will doch als Dev keinen überladenen Client haben mit Zeugs, das instanziiert wird, das ich nicht brauche; mich nur Leistung kostet.
Wenn ich "Status" brauche, dann instanziier ich auch nur den "AzureDevOpsStatusProvider" - nicht mehr, nicht weniger. Brauche ich zwei oder drei, dann eben die. Aber ich hab nicht einen Fall, wo wir mehr brauchen.

Wie gesagt; vermutlich auch dem geschuldet, dass unsere Logik-Implementierung durchaus als "sehr gut" zu bezeichnen ist, weil sehr modular und sehr schlank: immer Event- bzw. Use Case getrieben.
Ich glaub weiterhin, dass Du vielleicht einen Aufbau hast, der ungünstig ist, und Du daher "viele Provider" gleichzeitig injizieren musst, was in den meisten Fällen eben auf eine ungünstige Vorgehensweise in der Sofware Architektur hinweist (zB riesige Service-Klassen statt Umsetzung nach Use Cases).

Das Vorgehen hat zusätzlich den Vorteil, dass ich einzelne Provider auf eine andere API-Version (respektive API Urls) betreiben kann als andere Provider, zB um in gewissen Fällen neue/alte Features zu nutzen.
Das wäre mit Deinem Vorgehen, in dem es nur einen einzigen Client für alle gibt, nicht (ohne Zusatzaufwand) möglich.

Ja kann ich voll und ganz nachvollziehen.

Der "Client" soll auch nicht alles direkt instanziieren, sondern erst, wenn es gebraucht wird. Dafür will ich Lazy<T> nutzen, wozu es auch gedacht ist.

Ebenfalls könnte man die einzelnen Provider hernehmen, wenn man dies möchte. Auch ist die ApiVersion die genutzt wird, änderbar, wie im Code zu sehen.

Auch ist die ApiVersion die genutzt wird, änderbar, wie im Code zu sehen.

Finde ich bei Dir auch eher unglücklich.

Task<string> GetAllAsync(string? memberId, string? ownerId, string apiVersion = "7.0")

Wer sagt denn, dass die GetAll-Methode bei allen API Versionen a) die gleichen Parameter hat oder b) den gleichen Rückgabetyp.
Hinzu kommen verschiedene Arten von Auth oder Exceptions. Das ist alles mit so einem Vorgehen nicht handlebar.

Und nich vergessen: man wiederholt sich halt hart, wenn man bei jeder Methode die Version übergeben muss.

Es ist ja nur Feedback von mir: Du kannst das ruhig so tun. Ich hab mit solchen Konstrukten ausschließlich negative Erfahrung gemacht/mitbekommen und auch in 100% der Fälle eine andere Wurzel des Übels gesehen.
Vielleicht ist das bei Dir anders.


Edit weil vergessen:

Dafür will ich Lazy<T> nutzen, wozu es auch gedacht ist.

Das fällt unter "danger of syntactic sugar", weil u.a. enorm die Komplexität der Implementierung verschleiert wird.
Lazy-Properties kommen mit Nachteilen

Zitat von Abt

Wer sagt denn, dass die GetAll-Methode bei allen API Versionen a) die gleichen Parameter hat oder b) den gleichen Rückgabetyp.

Ist ein Punkt, an den ich noch nicht gedacht habe. Die Azure DevOps Api bietet ja jetzt schon zich Versionen an.
Hast du eine Empfehlung, sowas umzusetzen? Das man sich nicht immer wiederholt.

Also zusammenfassend, würdest du dazu tendieren, die einzelnen Provider zur Verfügung zu stellen?

Zitat von Duesmannr

Also zusammenfassend, würdest du dazu tendieren, die einzelnen Provider zur Verfügung zu stellen?

"Tendiert"? Also tendiert ist mit Abstand nicht ausreichend, wenn Du nochmal meine Beiträge liest 😄

Hast du eine Empfehlung, sowas umzusetzen? Das man sich nicht immer wiederholt.

Kannst quasi nicht wiederholungsfrei umsetzen. Jede API Version hat ihr eigenes Refit-Interface, respektive den eigenen Provider.
Natürlich kannst Dir über Source Code Generatoren sehr viel abnehmen lassen; aber C# ist halt nun mal typisiert ergo musst damit umgehen.