Ninject factory management

Question

I'm using Ninject.Extensions.Factory to control the lifecycle of the repository layer. I want to have a single point of reference from which I can get a reference to all repositories and have them lazily available. Ninject Factory approach seems like a good solution but I'm not too sure about my solution:

public class PublicUow : IPublicUow
{
    private readonly IPublicRepositoriesFactory _publicRepositoriesFactory;

    public PublicUow(IPublicRepositoriesFactory publicRepositoriesFactory)
    {
        _publicRepositoriesFactory = publicRepositoriesFactory;
    }

    public IContentRepository ContentRepository { get { return _publicRepositoriesFactory.ContentRepository; } }
    public ICategoryRepository CategoryRepository { get { return publicRepositoriesFactory.CategoryRepository; } }
}

The problem lies in the PublicRepositories class.

public class PublicRepositoriesFactory : IPublicRepositoriesFactory
{
    private readonly IContentRepositoryFactory _contentRepositoryFactory;
    private readonly ICategoryRepositoryFactory _categoryRepositoryFactory;

    public PublicRepositoriesFactory(IContentRepositoryFactory contentRepositoryFactory, ICategoryRepositoryFactory categoryRepositoryFactory)
    {
        _contentRepositoryFactory = contentRepositoryFactory;
        _categoryRepositoryFactory = categoryRepositoryFactory;
    }

    public IContentRepository ContentRepository { get { return _contentRepositoryFactory.CreateContentRepository(); } }
    public ICategoryRepository CategoryRepository { get { return _categoryRepositoryFactory.CreateCategoryRepository(); } }
}

I'm worried that this will become hard to manage as the number of repositories increases, this class might at some point need to have around 20-30 constructor arguments with the current implementation. Is there an approach I can take to reduce the number of ctr arguments, like passing an array/dictionary of interfaces or something similar?

I've thought about using property injection in this scenario but most articles suggest avoiding property injection in general.

Is there maybe a more general pattern that would make this easier to manage?

Is this in general a good approach?

Answer 1

It has become rather common practice to use a repository interface like

public interface IRepository
{
    T LoadById<T>(Guid id);

    void Save<T>(T entity);

    ....
}

instead of a plethora of specific repositories like IContentRepository, ICategoryRepository,..

specific repositories are only ever useful in case of having specific logic to the entity type and an operation, for example verifying that it's valid. But such operations are rather an "aspect" or a cut-through-concern which you should model as such. Managing/doing validation on save should not be implemented x-times but only once. The only thing you should specifically implement are the exact validation rules (DRY). But these should be implemented in separate classes and used by composition, not inheritance.

Also, for stuff like retrieving an entity or multiple entities "based on a use case", you should use specific query classes, and not put methods on a repository interface (SRP, SOC). An example would be GetProductsByOrder(Guid orderId). This should be neither on the Products nor the Order Repository but rather in a separate class itself.

Taking things a step further, it does not seem a good idea to use a factory to late create all repositories. Why?

• makes software more complex (thus harder to maintain and extend)
• usually negligible performance gain
• deteriorates testability
• also see Mark Seeman's blog post Service Locator is an anti pattern, where he's also talking about the disadvantages of late-creation vs. the composition of the entire object graph in one go.
• I'm not trying to say that you should never use factory/lazy, but only when you've got a really good reason to :)

Example of a query

I'm not very familiar with EntityFramework. I know NHibernate a whole lot better, so behold.

public class GetParentCategoriesQuery : IGetParentCategoriesQuery
{
    private readonly EntityFrameworkContext context;

    public GetParentCategories(EntityFrameworkContext context)
    {
        this.context = context;
    }

    public IEnumerable<Category> GetParents(Category child)
    {
        return this.context.Categories.Where(x => x.Children.Contains(child));
    }
}

So basically the only thing you change is extracting the GetParentCategoriesQuery into it's own class. The DbContext instance must be shared with the other query and repository instances. For web projects, this is done by binding the DbContext .InRequestScope(). For other applications you may need to use another machanism.

The usage of the query would be quite simple:

public class CategoryController
{
   private readonly IRepository repository;
   private readonly IGetParentCategoriesQuery getParentCategoriesQuery;

   public CategoryController(
            IRepository repository, 
            IGetParentCategoriesQuery getParentCategoriesQuery)
   {
       this.repository = repository;
       this.getParentCategoriesQuery = getParentCategoriesQuery;
   }

   public void Process(Guid categoryId)
   {
       Category category = this.repository.LoadById(categoryId);
       IEnumerable<Category> parentCategories = 
           this.getParentCategoriesQuery(category);

       // so some stuff...
   }
}

An alternative to the scoping is to have the repository instantiate the the query type and pass the DbContext to the query instance (this can be done using the factory extensions):

public TQuery CreateQuery<TQuery>()
{
    return this.queryFactory.Create<TQuery>(this.context);
}

which would be used like:

IEnumerable<Category> parents = repository
    .CreateQuery<GetParentCategoriesQuery>()
    .GetParents(someCategory);

But please note that this alternative will again only late-create the query and thus result in less testability (binding issues may be remain undetected for longer).

The GetParentCategoriesQuery is part of the repository layer, but not part of the repository class.