Como implementar o princípio DRY ao usar a palavra-chave 'using'?

23

Considere estes métodos:

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

Usar bloco é o mesmo e foi repetido 3 vezes aqui (é claro, mais de 100 vezes no aplicativo real). Como é possível implementar o princípio DRY (Don't Repeat Yourself) para o usingbloco? É considerado uma violação do principal DRY?

Atualização: não estou falando sobre o que foi implementado dentro do usingbloco. O que eu realmente quero dizer aqui é o using (Entities entities = new Entities()). Esta linha é repetida 100 vezes ou mais.

Saeed Neamati
fonte
2
isso é c #? A resposta à sua pergunta poderia ser dependentes do idioma
David
Sim, David, desculpe por não ter mencionado meu idioma. Como isso pode afetar a resposta?
Saeed Neamati
alguns idiomas podem ter sintaxe específica que pode ajudar a fatorar um pouco do seu código. Eu não sei c #, mas em Ruby, acho que você poderia usar blocos para fatorar a parte de uso.
David
A instrução using realmente fornece suporte ao idioma C # para aplicar o princípio DRY para ajudar a evitar a codificação repetitiva ao gerenciar o descarte de recursos com o padrão de design Dispose . Isso não significa que não podemos encontrar maneiras de tornar as coisas mais secas! Pessoalmente, penso no DRY como um processo recursivo.
John Tobler

Respostas:

24

Uma idéia seria envolvê-lo com uma função que leva a Func.

Algo assim

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

Então o seu código acima se torna

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

Também fiz Entitiesum parâmetro de tipo, porque estou assumindo que você tem mais de um tipo com o qual está fazendo isso. Caso contrário, você pode removê-lo e apenas usar o parâmetro param para o tipo de retorno.

Para ser sincero, esse tipo de código não ajuda na legibilidade. Na minha experiência, os colegas de trabalho Jr também enfrentam dificuldades.

Atualizar Algumas variações adicionais nos ajudantes que você pode considerar

//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.
Ribeiro
fonte
1
Uma boa solução, embora não resolva o problema real (não incluído na pergunta original), ou seja, as muitas ocorrências de Entities.
Aug2
@ Doc Brown: Eu acho que eu vencê-lo com o nome da função. Eu deixei de IEnumerablefora da função caso houvesse alguma não- IEnumerablepropriedade de T que o chamador quisesse retornar, mas você está certo, isso o limparia um pouco. Talvez seja bom ter um ajudante para os solteiros e os IEnumerableresultados. Dito isto, eu ainda acho que retarda o reconhecimento de que o código faz, especialmente para alguém que não está acostumado a usar lotes de genéricos e lambdas (por exemplo, seus colegas de trabalho que não estão no SO :))
Brook
+1 Acho que essa abordagem é boa. E a legibilidade pode ser melhorada. Por exemplo, coloque "ToList" em WithEntities, use em Func<T,IEnumerable<K>>vez de Func<T,K>e atribua a "WithEntities" um nome melhor (como SelectEntities). E eu não acho que "Entidades" precise ser um parâmetro genérico aqui.
Doc Brown
1
Para fazer isso funcionar, as restrições precisariam ser where T : IDisposable, new(), conforme usingnecessário IDisposablepara funcionar.
Anthony Pegram
1
@ Anthony Pegram: Fixo, obrigado! (que é o que eu recebo para codificação C #, em uma janela do navegador)
Brook
23

Para mim, isso seria como se preocupar em procurar a mesma coleção várias vezes: é apenas algo que você precisa fazer. Qualquer tentativa de abstraí-lo tornaria o código muito menos legível.

Ben Hughes
fonte
Grande analogia @Ben. +1
Saeed Neamati
3
Eu não concordo, desculpe. Leia os comentários do OP sobre o escopo da transação e pense no que você deve fazer quando escrever 500 vezes esse tipo de código e observe que precisará alterar a mesma coisa 500 vezes. Esse tipo de repetição de código pode ser aprovado apenas se você tiver <10 dessas funções.
Doc Brown
Ah, e para não esquecer, se você for cada um da mesma coleção mais de 10 vezes de uma maneira muito semelhante, com um código de aparência semelhante dentro do seu for-each, você deve definitivamente pensar em alguma generalização.
Doc Brown
1
Para mim, parece que você está transformando um lápis de linha em um delineador ... você ainda está se repetindo.
21430 Jim
Depende do contexto, se a foreachcoleção estiver muito grande ou se a lógica no foreachloop estiver demorando, por exemplo. Um lema que tem vindo a adotar: Não obsess mas sempre estar atento a sua abordagem
Coops
9

Parece que você está confundindo o princípio "Uma vez e apenas uma vez" com o princípio DRY. O princípio DRY afirma:

Todo conhecimento deve ter uma representação única, inequívoca e autoritativa dentro de um sistema.

No entanto, o princípio Uma vez e Somente uma vez é um pouco diferente.

O princípio [DRY] é semelhante ao OnceAndOnlyOnce, mas com um objetivo diferente. Com o OnceAndOnlyOnce, é recomendável refatorar para eliminar o código e a funcionalidade duplicados. Com o DRY, você tenta identificar a fonte única e definitiva de todo conhecimento utilizado em seu sistema e, em seguida, usa essa fonte para gerar instâncias aplicáveis ​​desse conhecimento (código, documentação, testes, etc.).

O princípio DRY é geralmente usado no contexto da lógica real, não tão redundante usando instruções:

Manter a estrutura de um programa SECO é mais difícil e tem menor valor. São as regras de negócios, as instruções if, as fórmulas matemáticas e os metadados que devem aparecer em apenas um lugar. O material WET - páginas HTML, dados de teste redundantes, vírgulas e {} delimitadores - é fácil de ignorar; portanto, SECAR é menos importante.

Fonte

Phil
fonte
7

Não vejo o uso de usingaqui:

E se:

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

Ou melhor ainda, pois acho que você não precisa criar um novo objeto todas as vezes.

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

Quanto à violação de DRY: DRY não se aplica a este nível. De fato, nenhum princípio realmente existe, exceto o da legibilidade. Tentar aplicar o DRY nesse nível é realmente apenas uma micro-otimização arquitetônica, que, como toda micro-otimização, é apenas uma troca de bicicletas e não soluciona nenhum problema, mas corre o risco de introduzir novos.
Pela minha própria experiência, sei que se você tentar reduzir a redundância de código nesse nível, criará um impacto negativo na qualidade do código, ofuscando o que era realmente claro e simples.

Edit:
Ok. Portanto, o problema não é realmente a instrução using, o problema é a dependência do objeto que você cria todas as vezes. Eu sugeriria injetar um construtor:

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
back2dos
fonte
2
Mas @ back2dos, existem muitos lugares em que usamos o using (CustomTransaction transaction = new CustomTransaction())bloco de código em nosso código para definir o escopo de uma transação. Isso não pode ser agrupado em um único objeto e, em todos os lugares em que você deseja usar uma transação, você deve escrever um bloco. Agora, e se você quiser alterar o tipo dessa transação de CustomTransactionpara BuiltInTransactionem mais de 500 métodos? Isso me parece uma tarefa repetida e um exemplo de violação do principal DRY.
Saeed Neamati
3
Ter "using" aqui fecha o contexto dos dados, de modo que o carregamento lento não é possível fora desses métodos.
Steven Striga 26/08
@ Saeed: É quando você olha para a injeção de dependência. Mas isso parece ser bem diferente do caso, como afirmado na pergunta.
um CVn
@Saeed: Postagem atualizada.
Aug2
@WeekendWarrior using(neste contexto) ainda é uma "sintaxe conveniente" mais desconhecida? Por que é tão bom usar =)
Coops
4

Não apenas o uso é de código duplicado (a propósito, ele é código duplicado e, na verdade, se compara a uma instrução try..catch..finally), mas também a toList. Eu refatoraria seu código assim:

 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));
}
Cohen
fonte
3

Como não há lógica comercial de qualquer tipo aqui, exceto a última. Não é realmente SECO, na minha opinião.

O último não possui DRY no bloco using, mas acho que a cláusula where deve mudar onde quer que seja usada.

Este é um trabalho típico para geradores de código. Escreva e cubra o gerador de código e deixe-o gerar para cada tipo.

arunmur
fonte
Não @arunmur. Houve um mal-entendido aqui. Com DRY, eu quis dizer using (Entities entities = new Entities())bloqueio. Quero dizer, essa linha de código é repetida 100 vezes e está sendo repetida cada vez mais.
Saeed Neamati
O DRY vem principalmente do fato de você precisar escrever casos de teste várias vezes e um bug em um local significa que você precisa corrigi-lo em 100 locais. O uso (Entidades ...) é um código muito simples de quebrar. Ele não precisa ser dividido ou colocado em outra classe. Se você ainda insistir em simplificá-lo. Eu sugeriria uma função de retorno de chamada Yeild do ruby.
Arunmur
1
@arnumur - o uso nem sempre é simples de quebrar. Costumo ter uma boa lógica que determina quais opções usar no contexto dos dados. É muito possível que a cadeia de conexão errada pode ser passado.
Morgan Herlocker
1
@ironcode - Nessas situações, faz sentido transferi-lo para uma função. Mas nesses exemplos é bastante difícil de quebrar. Mesmo que isso ocorra, a correção geralmente está na definição da própria classe Entities, que deve ser coberta com um caso de teste separado.
Arunmur
+1 @arunmur - eu concordo. Eu costumo fazer isso sozinho. Eu escrevo testes para essa função, mas parece um pouco exagerado escrever um teste para sua instrução using.
Morgan Herlocker
2

Como você está criando e destruindo o mesmo objeto descartável repetidamente, sua própria classe é um bom candidato para implementar o padrão 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();
    }
}

Isso deixa você apenas precisando do "using" ao criar uma instância da sua classe. Se você não deseja que a classe seja responsável por descartar os objetos, faça com que os métodos aceitem a dependência como argumento:

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

Minha parte favorita de magia incompreensível!

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

Wrapexiste apenas para abstrair isso ou qualquer mágica que você precisa. Não tenho certeza se recomendaria isso o tempo todo, mas é possível usá-lo. A idéia "melhor" seria usar um contêiner de DI, como StructureMap, e apenas colocar a classe Entities no contexto da solicitação, injetá-la no controlador e, então, cuidar do ciclo de vida sem a necessidade de seu controlador.

Travis
fonte
Os parâmetros de seu tipo para Func <IEnumerable <T>, Entities> estão na ordem errada. (ver minha resposta, que tem basicamente a mesma coisa)
Brook
Bem, fácil o suficiente para corrigir. Eu nunca me lembro qual é a ordem certa. Eu uso Funco suficiente.
Travis