Come implementare il principio DRY quando si utilizza la parola chiave 'using'?

23

Considera questi metodi:

public List<Employee> GetAllEmployees()
{
    using (Entities entities = new Entities())
    {
        return entities.Employees.ToList();
    }
}

public List<Job> GetAllJobs()
{
    using (Entities entities = new Entities())
    {
        return entities.Jobs.ToList();
    }
}

public List<Task> GetAllTasksOfTheJob(Job job)
{
    using (Entities entities = new Entities())
    {
        return entities.Tasks.Where(t => t.JobId == job.Id).ToList();
    }
}

L'uso del blocco è lo stesso ed è stato ripetuto 3 volte qui (ovviamente più di 100 volte nell'applicazione reale). Come è possibile implementare il principale DRY (Do not Repeat Yourself) per using block? È considerata una violazione del principale di DRY?

Aggiornamento: non sto parlando di ciò che è stato implementato all'interno del blocco using . Quello che in realtà intendo qui è il using (Entities entities = new Entities()) . Questa riga viene ripetuta 100 volte o più.

    
posta Saeed Neamati 26.08.2011 - 14:06
fonte

8 risposte

24

Un'idea sarebbe quella di avvolgerla con una funzione che prende un Func .

Qualcosa di simile

public K UsingT<T,K>(Func<T,K> f) where T:IDisposable,new()
{
    using (T t = new T())
    {
        return f(t);
    }
}

Quindi il tuo codice precedente diventa

public List<Employee> GetAllEmployees()
{
    return UsingT<Entities,List<Employee>>(e=>e.Employees.ToList());
}

public List<Job> GetAllJobs()
{
    return UsingT<Entities,List<Job>>(e=>e.Jobs.ToList());
}

public List<Task> GetAllTasksOfTheJob(Job job)
{
    return UsingT<Entities,List<Task>>(e=>e.Tasks.Where(t => t.JobId == job.Id).ToList());
}

Ho creato Entities anche un parametro di tipo, perché presumo che tu abbia più di un tipo con cui stai facendo questo. Se non lo sei, puoi rimuoverlo e utilizzare solo il tipo param per il tipo restituito.

Ad essere onesti, questo tipo di codice non aiuta affatto la leggibilità. Nella mia esperienza, più collaboratori di Jr hanno davvero un momento difficile con esso.

Aggiorna Alcune varianti aggiuntive sugli helper che potresti prendere in considerazione

//forget the Entities type param
public T UsingEntities<T>(Func<Entities,T> f)
{
    using (Entities e = new Entities())
    {
        return f(e);
    }
}
//forget the Entities type param, and return an IList
public IList<T> ListFromEntities<T>(Func<Entities,IEnumerable<T>> f)
{
    using (Entities e = new Entities())
    {
        return f(e).ToList();
    }
}
//doing the .ToList() forces the results to enumerate before 'e' gets disposed.
    
risposta data 26.08.2011 - 14:59
fonte
23

Per me sarebbe come preoccuparsi di dover utilizzare più volte la stessa raccolta: è solo qualcosa che devi fare. Qualsiasi tentativo di astrazione ulteriormente renderebbe il codice molto meno leggibile.

    
risposta data 26.08.2011 - 14:54
fonte
9

Sembra che tu stia confondendo il principio "Once and Only Once" con il principio DRY. Il principio DRY afferma:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Tuttavia il principio Once e Only Once è leggermente diverso.

[The DRY] principle is similar to OnceAndOnlyOnce, but with a different objective. With OnceAndOnlyOnce, you are encouraged to refactor to eliminate duplicated code and functionality. With DRY, you try to identify the single, definitive source of every piece of knowledge used in your system, and then use that source to generate applicable instances of that knowledge (code, documentation, tests, etc).

Il principio DRY viene solitamente utilizzato nel contesto della logica effettiva, non tanto per le istruzioni ridondanti:

Keeping the structure of a program DRY is harder, and lower value. It's the business rules, the if statements, the math formulas, and the metadata that should appear in only one place. The WET stuff - HTML pages, redundant test data, commas and {} delimiters - are all easy to ignore, so DRYing them is less important.

Source

    
risposta data 26.08.2011 - 17:47
fonte
7

Non riesco a vedere l'uso di using qui:

Che ne dici di:

public List<Employee> GetAllEmployees() {
    return (new Entities()).Employees.ToList();
}
public List<Job> GetAllJobs() {
    return (new Entities()).Jobs.ToList();
}
public List<Task> GetAllTasksOfTheJob(Job job) {
    return (new Entities()).Tasks.Where(t => t.JobId == job.Id).ToList();
}

O anche meglio, poiché non penso che sia necessario creare ogni volta un nuovo oggetto.

private Entities entities = new Entities();//not sure C# allows for that kind of initialization, but you can do it in the constructor if needed

public List<Employee> GetAllEmployees() {
    return entities.Employees.ToList();
}
public List<Job> GetAllJobs() {
    return entities.Jobs.ToList();
}
public List<Task> GetAllTasksOfTheJob(Job job) {
    return entities.Tasks.Where(t => t.JobId == job.Id).ToList();
}

Per quanto riguarda la violazione di DRY: DRY non si applica a questo livello. In effetti nessun principio lo fa davvero, tranne quello di leggibilità. Cercare di applicare DRY a quel livello è in realtà solo una micro-ottimizzazione dell'architettura, che come tutte le micro-ottimizzazione è solo eliminazione delle biciclette e non risolve problemi, ma rischia anche di introdurne di nuovi.
Dalla mia esperienza personale, so che se provi a ridurre la ridondanza del codice a quel livello, crei un impatto negativo sulla qualità del codice, offuscando ciò che era veramente chiaro e semplice.

Modifica
Ok. Quindi il problema non è in realtà l'istruzione using, il problema è la dipendenza dall'oggetto che si crea ogni volta. Suggerirei di iniettare un costruttore:

private delegate Entities query();//this should be injected from the outside (upon construction for example)
public List<Employee> GetAllEmployees() {
    using (var entities = query()) {//AFAIK C# can infer the type here
        return entities.Employees.ToList();
    }
}
//... and so on
    
risposta data 26.08.2011 - 14:22
fonte
4

Non solo l'uso è un codice duplicato (dal modo in cui è un codice duplicato e in realtà è paragonabile a una frase try..catch..finally) ma anche l'elenco. Vorrei refactoring il tuo codice in questo modo:

 public List<T> GetAll(Func<Entities, IEnumerable<T>> getter) {
    using (Entities entities = new Entities())
    {
        return getter().ToList();
    }
 }

public List<Employee> GetAllEmployees()
{
    return GetAll(e => e.Employees);
}

public List<Job> GetAllJobs()
{
    return GetAll(e => e.Jobs);
}

public List<Task> GetAllTasksOfTheJob(Job job)
{
    return GetAll(e => e.Tasks.Where(t => t.JobId == job.Id));
}
    
risposta data 26.08.2011 - 19:38
fonte
3

Poiché non vi è alcuna logica di business di alcun tipo qui tranne che per l'ultimo. Non è proprio ASCIUTTO, a mio avviso.

L'ultimo non ha DRY nel blocco using, ma suppongo che la clausola where debba cambiare dove mai è stata usata.

Questo è un tipico lavoro per i generatori di codice. Scrivi e copri il generatore di codice e fallo generare per ogni tipo.

    
risposta data 26.08.2011 - 14:21
fonte
2

Dato che stai creando e distruggendo lo stesso oggetto usa e getta più e più volte, la tua classe è di per sé un buon candidato per l'implementazione del modello IDisposable.

class ThisClass : IDisposable
{
    protected virtual Entities Context { get; set; }

    protected virtual void Dispose( bool disposing )
    {
        if ( disposing && Context != null )
            Context.Dispose();
    }

    public void Dispose()
    {
        Dispose( true );
    }

    public ThisClass()
    {
        Context = new Entities();
    }

    public List<Employee> GetAllEmployees()
    {
        return Context.Employees.ToList();
    }

    public List<Job> GetAllJobs()
    {
        return Context.Jobs.ToList();
    }

    public List<Task> GetAllTasksOfTheJob(Job job)
    {
        return Context.Tasks.Where(t => t.JobId == job.Id).ToList();
    }
}

Questo ti lascia solo bisogno di "usare" quando crei un'istanza della tua classe. Se non vuoi che la classe sia responsabile della disposizione degli oggetti, allora puoi fare in modo che i metodi accettino la dipendenza come argomento:

public static List<Employee> GetAllEmployees( Entities entities )
{
    return entities.Employees.ToList();
}

public static List<Job> GetAllJobs( Entities entities )
{
    return entities.Jobs.ToList();
}

public static List<Task> GetAllTasksOfTheJob( Entities entities, Job job )
{
    return entities.Tasks.Where(t => t.JobId == job.Id).ToList();
}
    
risposta data 27.08.2011 - 17:07
fonte
1

Il mio pezzo preferito di magia senza compromessi!

public class Blah
{
  IEnumerable<T> Wrap(Func<Entities, IEnumerable<T>> act)
  {
    using(var entities = new Entities()) { return act(entities); }
  }

  public List<Employee> GetAllEmployees()
  {
    return Wrap(e => e.Employees.ToList());
  }

  public List<Job> GetAllJobs()
  {
    return Wrap(e => e.Jobs.ToList());
  }

  public List<Task> GetAllTasksOfTheJob(Job job)
  {
    return Wrap(e => e.Tasks.Where(x ....).ToList());
  }
}

Wrap esiste solo per sottrarre quella magia o qualsiasi altra cosa di cui hai bisogno. Non sono sicuro di raccomandarlo tutto il tempo ma è possibile utilizzarlo. L'idea "migliore" sarebbe quella di utilizzare un contenitore DI, come StructureMap, e solo l'ambito della classe Entities per il contesto della richiesta, iniettarlo nel controller e lasciare che si occupi del ciclo di vita senza che sia necessario il controller.

    
risposta data 26.08.2011 - 18:09
fonte

Leggi altre domande sui tag