Is it bad practice that a controller calls a repository instead of a service?
To explain more:
I figure out that in good design controllers call services and services use repositories.
But sometimes in controller I don't have/need any logic and just need to fetch from the database and pass it to the view.
And I can do it by just calling the repository - no need to call the service. Is this bad practice?
No, think of it this way: a repository is a service (also).
If the entities you retrieve through the repository handle most of the business logic there is no need for other services. Just having the repository is enough.
Even if you have some services that you must pass through to manipulate your entities. Grab the entity from the repository first and then pass it to said service. Being able to toss up a HTTP 404 before even trying is very convenient.
Also for read scenario's it's common you just need the entity to project it to a DTO/ViewModel. Having a service layer in between then often results in a lot of pass through methods which is rather ugly.
It is not bad practice for a controller to call a repository directly. A "service" is just another tool, so use it where it makes sense.
NikolaiDante commented:
... Pick the right pattern for the right application. What I would say is that you should make your application consistent.
I don't think consistency is the most important aspect. A "service" class is meant to encapsulate some higher level logic so the controller doesn't need to implement it. If there is no "higher level logic" required for a given operation, just go directly to the repository.
To promote good Separate of Concerns and testability, the repository should be a dependency you inject into the service via a constructor:
IFooRepository repository = new FooRepository();
FooService service = new FooService(repository);
service.DoSomething(...);
If searching for records in the database needs some sort of parameterized query, a service class might be a good place to take in your view model and build a query that is then executed by the repository.
Similarly, if you have a complex view model for a form, a service class can encapsulate the logic of creating, updating and deleting records by calling methods on your Domain Models/Entities, then persisting them using a repository.
Going the opposite direction, if your controller needs to get a record by its Id, then delegating to a service object for this is like hitting a thumbtack with a sledgehammer -- it's way more than you need.
I have found that the controller is in the best position to handle the transaction, or a Unit Of Work object. The controller or Unit Of Work object would then delegate to service objects for complex operations, or go directly to the repository for simple operations (like finding a record by Id).
public class ShoppingCartsController : Controller
{
[HttpPost]
public ActionResult Edit(int id, ShoppingCartForm model)
{
// Controller initiates a database session and transaction
using (IStoreContext store = new StoreContext())
{
// Controller goes directly to a repository to find a record by Id
ShoppingCart cart = store.ShoppingCarts.Find(id);
// Controller creates the service, and passes the repository and/or
// the current transaction
ShoppingCartService service = new ShoppingCartService(store.ShoppingCarts);
if (cart == null)
return HttpNotFound();
if (ModelState.IsValid)
{
// Controller delegates to a service object to manipulate the
// Domain Model (ShoppingCart)
service.UpdateShoppingCart(model, cart);
// Controller decides to commit changes
store.SaveChanges();
return RedirectToAction("Index", "Home");
}
else
{
return View(model);
}
}
}
}
I think a mix of services and working with repositories directly is perfectly acceptable. You could further encapsulate the transaction in a Unit Of Work object if you felt the need.
The breakdown of responsibilities goes like this:
It depends on your architecture. I use Spring, and transactionality is always managed by services.
If you call repositories directly for write operations (Or simple services without logic that simply delegate to the repository), probably you are using several database transactions for an operation that must be done in one. That will lead to incoherent data in your database. As a general rule, database operations should work, or should fail, but half-working operations are the cause of headaches.
For that reason I think that calling repositories directly from controllers, or using simple delegating services, is a bad practice. You begin doing it just for reading, and very soon you, or one or your mates will start doing it for write operations.
Yes, it is a bad practice. Controller should be used only to control the flow of your application, get input from client apps, call service and pass data to the view (JSON/HTML/XML/etc). Repository/DAO should be used to retrieve/persist the data without knowing any business logic.
Consider following scenarios:
