È una buona opzione di progettazione per chiamare un metodo di inizializzazione di classe all'interno di una fabbrica prima di iniettarlo

Question

Sono in procinto di refactoring una porzione piuttosto grande di codice spaghetti. In poche parole è una grande classe "divina" che si dirama in due processi diversi a seconda delle condizioni. Entrambi i processi sono lunghi e hanno un sacco di codice duplicato.

Quindi il mio primo sforzo è stato quello di estrarre questi due processi nelle proprie classi e inserire il codice comune in un genitore da cui entrambi ereditano.

Sembra qualcosa di simile a questo:

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(); 
    } 
} 

io sono un avido lettore del blog di Mark Seemann e in this entry spiega che questo codice ha un odore di accoppiamento temporale dal momento che è necessario chiamare il metodo Load sul fornitore di dati prima che sia in uno stato utilizzabile.

Sulla base di questo, e dal momento che l'oggetto viene iniettato a quelli restituiti dalla fabbrica in ogni caso, sto pensando di cambiare la fabbrica per fare questo:

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

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

a causa del nome "DataProvider" si probabilmente ho indovinato che il metodo Load sta effettivamente facendo un accesso al database.

Qualcosa mi dice che un oggetto che fa un accesso al database all'interno del metodo di creazione di una fabbrica astratta non è un buon progetto.

Ci sono linee guida, buone pratiche o qualcosa che dice che questa è effettivamente una cattiva idea?

Grazie per il vostro aiuto.

Answer 1

In genere, una fabbrica viene utilizzata per risolvere tipi concreti di un'interfaccia richiesta o di un tipo astratto, in modo da poter separare i consumatori dall'implementazione. Di solito, una fabbrica sta solo scoprendo o specificando il tipo concreto, aiuta a risolvere le dipendenze e istanzia il tipo concreto e lo restituisce. Tuttavia, non esiste una regola dura o veloce su cosa può o non può fare, ma è ragionevole dare accesso sufficiente alle sole risorse necessarie per risolvere e istanziare tipi concreti.

Un altro buon uso di una fabbrica è quello di nascondere ai consumatori tipi di dipendenze che non sono rilevanti per il consumatore. Ad esempio, sembra che IExportDataProvider sia rilevante solo internamente e possa essere estratto dai consumatori (ad esempio ExportProcess).

Un odore di codice nell'esempio, tuttavia, è come viene utilizzato IExportDataProvider. Il modo in cui attualmente sembra funzionare, si ottiene un'istanza di esso una volta, ma è possibile modificare il suo stato in utilizzi successivi (chiamando Load). Ciò può causare problemi con la concorrenza e lo stato danneggiato. Poiché non so cosa faccia quel tipo o come sia effettivamente utilizzato dal tuo IExporter, è difficile dare una raccomandazione. Nel mio esempio qui di seguito, eseguo una rettifica in modo da poter presumere che il provider sia stateless e che invece Load restituisca una sorta di oggetto stato che la factory può utilizzare per risolvere il tipo concreto di esportatore e quindi fornire dati ad esso. Puoi regolarlo come meglio credi. D'altra parte, se il provider deve essere stato, è necessario creare un IExportDataProviderFactory, utilizzarlo nella fabbrica di esportazione e creare una nuova istanza del provider dalla fabbrica per ogni chiamata allo Create della factory di esportazione.

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); 
    } 
} 

E quindi consumare:

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. 
    } 
}