Ist es eine gute Design-Option, eine Initialisierungsmethode einer Klasse innerhalb einer Fabrik zu nennen, bevor sie injiziert wird?

Question

Ich bin gerade dabei, eine ziemlich große Portion Spaghetti-Code zu refaktorieren. Kurz gesagt ist es eine große "Gott-ähnliche" Klasse, die sich in zwei verschiedene Prozesse verzweigt, abhängig von einer bestimmten Bedingung. Beide Prozesse sind langwierig und haben viel doppelten Code.

Meine erste Bemühung bestand also darin, diese beiden Prozesse in ihre eigenen Klassen zu extrahieren und den gemeinsamen Code in ein Elternteil zu setzen, von dem beide erben.

Es sieht ungefähr so ​​aus:

public class ExportProcess 
{ 
    public ExportClass(IExportDataProvider dataProvider, IExporterFactory exporterFactory) 
    { 
     _dataProvider = dataProvider; 
     _exporterFactory = exporterFactory; 
    } 

    public void DoExport(SomeDataStructure someDataStructure) 
    { 
     _dataProvider.Load(someDataStructure.Id); 

     var exporter = _exporterFactory.Create(_dataProvider, someDataStructure); 

     exporter.Export(); 
    } 
} 

Ich bin ein begeisterter Leser von Mark Seemann Blog und in this entry erklärt er, dass dieser Code eine zeitliche Kopplung Geruch hat, da es notwendig ist, um die Load-Methode auf das anrufen Datenanbieter, bevor es in einem verwendbaren Zustand ist.

auf dieser Grundlage, und da das Objekt auf die ohnehin durch die Fabrik zurück diejenigen injiziert wird, ich denke, die Fabrik zu ändern, dies zu tun:

public IExporter Create(IExportDataProvider dataProvider, SomeDataStructure someDataStructure) 
{ 
    dataProvider.Load(someDataStructure.Id); 

    if(dataProvider.IsNewExport) 
    { 
     return new NewExportExporter(dataProvider, someDataStructure); 
    } 
    return new UpdateExportExporter(dataProvider, someDataStructure); 
} 

Wegen des Namens „Data Provider“ Sie wahrscheinlich vermutet, dass die Load-Methode tatsächlich einen Datenbankzugriff durchführt.

Etwas sagt mir, dass ein Objekt, das einen Datenbankzugriff innerhalb der create Methode einer abstrakten Fabrik macht, kein gutes Design ist.

Gibt es irgendwelche Richtlinien, Best Practices oder etwas, das besagt, dass dies effektiv eine schlechte Idee ist?

Danke für Ihre Hilfe.

Answer 1

In der Regel wird eine Factory verwendet, um konkrete Typen einer angeforderten Schnittstelle oder eines abstrakten Typs aufzulösen, sodass Sie die Verbraucher von der Implementierung entkoppeln können. Normalerweise wird eine Fabrik nur den konkreten Typ entdecken oder spezifizieren, Abhängigkeiten auflösen und den konkreten Typ instantiieren und zurückgeben. Es gibt jedoch keine feste oder schnelle Regel, was es tun kann oder was nicht, aber es ist sinnvoll, es nur auf Ressourcen zu beschränken, die es braucht, um konkrete Typen aufzulösen und zu instantiieren.

Eine andere gute Verwendung einer Fabrik ist es, vor Verbraucher Typen Abhängigkeiten zu verbergen, die für den Verbraucher nicht relevant sind. Zum Beispiel scheint es, IExportDataProvider ist nur intern relevant, und kann von den Verbrauchern abstrahiert werden (wie ExportProcess).

Ein Code-Geruch in Ihrem Beispiel ist jedoch, wie IExportDataProvider verwendet wird. Wie es derzeit zu funktionieren scheint, erhalten Sie einmal eine Instanz davon, aber es ist möglich, den Status in nachfolgenden Verwendungen zu ändern (durch Aufruf von Load). Dies kann zu Problemen mit Nebenläufigkeit und beschädigtem Zustand führen. Da ich nicht weiß, was dieser Typ macht oder wie er von Ihrer IExporter tatsächlich verwendet wird, ist es schwierig, eine Empfehlung abzugeben. In meinem Beispiel unten nehme ich eine Anpassung vor, so dass wir davon ausgehen können, dass der Anbieter zustandslos ist, und stattdessen Load eine Art Zustandsobjekt zurückgibt, das die Factory verwenden kann, um den konkreten Typ des Exporteurs aufzulösen, und dann Daten zur Verfügung stellt. Sie können das anpassen, wie Sie es für richtig halten. Wenn der Anbieter dagegen zustandsbehaftet sein muss, müssen Sie eine IExportDataProviderFactory erstellen, sie in Ihrer Exportfactory-Factory verwenden und für jeden Aufruf der Factory-Funktion Create des Herstellers eine neue Instanz des Providers erstellen.

public interface IExporterFactory 
{ 
    IExporter Create(SomeDataStructure someData); 
} 

public class MyConcreteExporterFactory : IExporterFactory 
{ 
    public MyConcreteExporterFactory(IExportDataProvider provider) 
    { 
      if (provider == null) throw new ArgumentNullException(); 

      Provider = provider; 
    } 

    public IExportDataProvider Provider { get; private set; }  

    public IExporter Create(SomeDataStructure someData) 
    { 
     var providerData = Provider.Load(someData.Id); 

     // do whatever. for example... 
     return providerData.IsNewExport ? new NewExportExporter(providerData, someData) : new UpdateExportExporter(providerData, someData); 
    } 
} 

Und dann verbrauchen:

public class ExportProcess 
{ 
    public ExportProcess(IExporterFactory exporterFactory) 
    { 
     if (exporterFactory == null) throw new ArgumentNullException(); 

     _exporterFactory = factory; 
    } 

    private IExporterFactory _exporterFactory; 

    public void DoExport(SomeDataStructure someData) 
    { 
     var exporter = _exporterFactory.Create(someData); 
     // etc. 
    } 
}