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.
... 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);
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
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)
// Controller delegates to a service object to manipulate the
// Domain Model (ShoppingCart)
// Controller decides to commit changes
return RedirectToAction("Index", "Home");
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:
- The controller controls the flow of the application
- Returns "404 Not Found" if the shopping cart isn't in the database
- Re-renders the form with validation messages if validation fails
- Saves the shopping cart if everything checks out
- The controller delegates to a service class to execute business logic on your Domain Models (or Entities). Service objects should not implement business logic! They execute business logic.
- Controllers may delegate directly to repositories for simple operations
- Service objects take data in the view model, and delegate to Domain Models to execute business logic (e.g. the service object calls methods on the Domain Models before calling methods on the repository)
- Service objects delegate to repositories for data persistence
- Controllers should either:
- Manage the lifetime of a transaction, or
- Create a Unit Of Work object to manage the lifetime of a transaction