2012-04-03 10 views
5

tengo el siguiente repositorio ADO .NetIDisposable en un repositorio inyectada

public class Repository : IRepository, IDisposable 
{ 
    private readonly IUnitOfWork UnitOfWork; 
    private SqlConnection Connection; 

    public Repository(IUnitOfWork unitOfWork, connectionString) 
    { 
     UnitOfWork = unitOfWork; 
     Connection = new SqlConnection(connectionString); 
     Connection.Open(); 
    } 

    public MyObject FindBy(string userName) 
    { 
     //...Ado .Net command.ExecuteReader, etc. 
    } 
} 

Este repositorio se inyecta con un contenedor IoC a un Servicio de dominio y se usa de esta manera:

public class UserDomainService : IUserDomainService 
{ 
    private readonly IRepository Repository; 

    public UserDomainService(IRepository repository) 
    { 
     Repository = repository; 
    } 

    public User CreateNewUser(User user) 
    { 
     using(Repository) 
     { 
     var user = Repository.FindBy(user.UserName); 
     if(user != null) 
      throw new Exception("User name already exists!"); 

     Repository.Add(user); 
     Repository.Commit(); 
     } 
    } 
} 

La idea es que siempre coloco el objeto Repository en una instrucción using para que cuando termine, la conexión se cierre y elimine, pero lo veo como un problema ya que la clase Domain Service todavía está activa y si hay una segunda llamada a la misma, fallará ya que el repositorio ya ha sido destruido.

Ahora tengo control total de todo el código y quiero diseñar solo llamadas de servicio de grano grueso, pero hay algo sobre todo lo que no se siente bien.

Lo estoy haciendo así, así puedo evitar que el Servicio de dominio conozca los métodos OpenConnection y CloseConnection en el repositorio.

¿Este diseño es inherentemente malo o hay una mejor manera de hacerlo?

Después de pensamiento: se está generando Todo el árbol de dependencias a nivel WCF cuando llega una petición y, por supuesto, se puede ver que la conexión se abre en ese momento, ya que sucede en el constructor del repositorio, por lo que creo que no es tan malo, ya que está abierto solo durante la duración de esta llamada en particular. ¿Estoy en lo cierto sobre esta suposición o estoy haciendo algo terriblemente malo abriendo la conexión de base tan temprano en el proceso?

+0

¿'IRepository' está estrechamente relacionado con' Repository'? Es decir, ¿contiene los métodos, como 'Find'? Si es así, ¿esa interfaz implica 'IDisposable'? –

+0

Tengo mi propia pregunta que podría estar relacionada con esto: [ServiceContainer, IoC y objetos desechables] (http://stackoverflow.com/questions/556580/servicecontainer-ioc-and-disposable-objects). –

+1

¿Por qué necesita 'SqlConnection' en el' Repository'? Parece más como algo para su 'IUnitOfWork'. – Steven

Respuesta

8

Inyecte una fábrica que crea las instancias que necesita, no una instancia en sí.

Tome un IRepositoryFactory para que pueda crear un IRepository y deshacerse de él cada vez que lo use. De esta forma, ni el servicio de dominio ni la fábrica deberían ser desechables. Además, y lo que es más importante, mantienes el código abstracto al inyectar la implementación en lugar de codificarla.

public class UserDomainService : IUserDomainService 
{ 
    private readonly IRepositoryFactory RepositoryFactory; 

    public UserDomainService(IRepositoryFactory factory) 
    { 
     RepositoryFactory = factory; 
    } 

    public User CreateNewUser(User user) 
    { 
     using (IRepository repository = RepositoryFactory.Create()) 
     { 
     var user = repository.FindBy(user.UserName); 
     if(user != null) 
      throw new Exception("User name already exists!"); 

     repository.Add(user); 
     repository.Commit(); 
     } 
    } 
} 

No siempre tiene que inyectar el tipo que necesita. Al leer en Castle Windsor (cuya mentalidad es register-resolve-release), descubres que si quieres Resolver cosas en un tiempo indeterminado en la vida de la aplicación, se sugiere utilizar Type Factories.

Sabe que necesitará un Repositorio, pero no sabe cuando. En lugar de solicitar un repositorio, solicite algo que cree ellos. El nivel de abstracción se mantiene así y no ha filtrado ninguna implementación.

+0

DUH! No sé por qué no pensé en esto. Gracias. –

+0

@SergioRomero A veces solo necesitas dar un paso atrás y escuchar el problema con otras personas antes de llegar a la conclusión correcta. Lo hago todo el tiempo, es muy fácil quedar atrapado en círculos de diseño si estás metido hasta las rodillas todo el tiempo :-( –

+0

Ahora tienes una abstracción con filtraciones. Violación de la encapsulación. El servicio no necesita saber sobre la duración del repositorio. Reveló el secreto. La mejor solución sería reescribir el repositorio para que se haya abierto y cerrado la conexión para cada transacción. –

1

El problema que tiene es de propiedad. La clase UserDomainService no crea el IRepository, pero aún asume la propiedad de esa instancia, ya que la elimina.

La regla general es que el que crea un objeto debe destruirlo. En otras palabras, quien crea el objeto es el propietario, y el propietario debe destruir ese objeto.

Existen dos soluciones a su problema.

  1. Crear una IRepositoryFactory, como Adán explica con claridad.Un método CreateNewRepository() en una fábrica de este tipo comunicará claramente que la persona que llama obtiene la propiedad y debe disponer de ese repositorio creado.

  2. Deje que quien crea (e inyecta) ese repositorio se ocupe de la eliminación de ese repositorio. O lo hace manualmente en su servicio WCF o utiliza un marco IoC/DI. En caso de que use un marco DI, probablemente debería considerar una duración de Por solicitud web, o algo similar.

última nota, sus implementos IRepositoryIDisposable. Al elegir la solución 2, puede eliminar la interfaz IDisposable de IRepository, que oculta el hecho de que los recursos están involucrados desde la aplicación. Ocultar IDisposable de la aplicación es algo bueno, ya que esa interfaz es una abstracción con goteras. Ya lo ha encontrado, ya que al llamar al Dispose desde dentro de la aplicación, se rompe toda la aplicación.

Cuestiones relacionadas