Onion Architecture - Service Layer Responsibility

Question

I am learning Onion Architecture by Jeffrey Palermo for more than 2 weeks now. I have created a test project by following this tutorial. While studying I came across this question on SO. According to accepted answer, one person nwang suggests that Methods like GetProductsByCategoryId should not be in Repository and one the other hand Dennis Traub suggests that it is the responsibility of the Repository. What I am doing is :

I have a General Repository in Domain.Interface in which I have a method Find :

public interface IRepository<TEntity> where TEntity : class
{
     IEnumerable<TEntity> Find(Expression<Func<TEntity, bool>> filter = null);
     .......
     .......
     .......
}

Then I created a BaseRepository in Infrastucture.Data:

public class RepositoryBase<TEntity> : IRepository<TEntity> where TEntity : class
{
     internal readonly DbSet<TEntity> dbSet;
     public virtual IEnumerable<TEntity> Find(
            Expression<Func<TEntity, bool>> filter = null)
     {
            IQueryable<TEntity> query = dbSet;

            if (filter != null)
            {
                query = query.Where(filter);
            }
            return query.ToList();
     }
}

And I have a concrete repository in Infrastructure.Data

public class ProductRepository : RepositoryBase<Product>, IProductRepository
{
      public ProductRepository(MyDBContext context)
           : base(context)
       {         

       }
}

Now what I am doing in my Service Layer is Injecting Repository into Service and calling Repository.Find for methods like GetProductsByCategoryId. Like :

public class ProductService : IProductService
{
     private readonly IUnitOfWork _unitOfWork;
     private readonly IProductRepository _productRepository;

     public ProductService(IUnitOfWork unitOfWork, IProductRepository productRepository)
     {
          _unitOfWork = unitOfWork;
          _productRepository = productRepository;
     }

     public IList<Product> GetProductsByCategoryId(int CategoryId)
     {
          // At the moment, My code is like this:
          return _productRepository.Find(e => e.CategoryId == CategoryId).ToList();

          // My confusion is here. Am I doing it right or I need to take this code to 
          // ProductRepository and call _productRepositoy.GetProductsByCategoryId(CategoryId) here instead.
          // If I do this, then Service Layer will become more of a wrapper around repository. Isn't it?
          // My question is : What exactly will be the responsibility of the Service Layer in Onion Architecture?
         }
    }

Vitriol

Something seems a little off. UnitOfWork should wrap all your repositories. So your call to get repository should look like _unitOfWork.ProductRepository.Find(e => e.CategoryId == CategoryId).ToList(); – Playa

Yeah I saw this approach in many examples but then your unit of work will become a repository container with all repositories tightly coupled to unit of work. – Vitriol

A beautiful example of Repository with Unit of work is here : codeproject.com/Articles/543810/… – Vitriol

Answer 1

The way you designed your application is ok... but only if your service will come to handle other things than just wrap up the repository methods!

Always keep in mind the YAGNI principle that says:

Always implement things when you actually need them, never when you just foresee that you need them

Let's say that you have a user story that says that whenever a product description is not found in your DB, you should retreive it from somewhere else (calling an external service or something). Then it seems obvious that your ProductService will have to have a

private readonly IProductRepository _productRepository;

but also a

private readonly IProductDescriptionService _productDescriptionService;

In that situation it really makes sense to add a service layer on top of your repositories.

Tramroad

That's great. Actually I am not working on a real project at the moment. I am trying to learn Onion Architecture. That's why I want to the know proper way of handling complex scenarios. – Vitriol

Ok. I've made a bunch of Onion related answers on SO. You might find some of them interesting. – Tramroad

Answer 2

I find that sometimes things can get over abstracted for the sake of it and offer no real value. I would say that the structure in your example is fine and follows the pattern correctly. Your service layer, correctly, is acting to serve the needs of the client UI, it is loosely coupled to the data layer and contains any business logic needed to manipulate the data.

I always think it is more productive to start simple and build upon your structure than it is to over abstract, over complicate and over bloat a project. A business or technical case will often drive the project, and dictate whether it is needed.

Accusation

That's a wonderful suggestion to start simple and thanks for the appreciation. Actually my concrete repository i.e. 'ProductRepository' does not have anything inside it which makes me worry and doubtful of the existence concrete repositories in my project. – Vitriol

You can use Repository<T> as a concrete class, if you'll follow this approach. This is a generic repository pattern – Skit

Answer 3

Although, in this case it seems that service later is just a wrapper, sometimes you might have the need to add some business logic or call two repositories. Lets say you have a service called CartService and you have a method called AddToCart in which you need to first get the product, do some calculation and then call insert to another repository like below.

public class CartService : ICartService 
{
   private readonly IUnitOfWork _unitOfWork;

   public CartService(IUnitOfWork unitOfWork)
   {
      _unitOfWork = unitOfWork;
   }

   public void AddToCart (int productId, int quantity)
   {
     var product = _unitOfWork.ProductRepository
                              .Find(p => p.ProductId == productId).Single();

     var cartItem = new CartItem { 
                    ProductId = productId, 
                    Desc = product.Desc, 
                    Quantity = quantiry 
                  };

     _unitOfWork.CartRepository.Add(cartItem);  
  }
}

More, complex scenarios include calling a third party web service etc.

Playa