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?

  • How are you calling the service? Through a REST interface? Commented Jan 8, 2016 at 17:25
  • 3
    I use that design approach myself rather commonly. My controller (or an underlying composer class) will request data from or send data to the repo, and then pass it to any service classes that need to do processing. No reason to combine data processing classes with data retrieval/management classes, they're different concerns though I know the typical approach is to do it that way. Commented Jan 8, 2016 at 17:27
  • 5
    Meh. If it's a small application and you're just trying to get data from a database, a service layer is a waste of time unless that service layer is part of some public API such as a REST interface. "Is milk good for you or bad for you?" Depends on whether you're lactose intolerant. Commented Jan 8, 2016 at 17:29
  • 5
    There isn't a hard and fast rule that you should have a Controller -> Service -> Repository structure over Controller -> Repository. Pick the right pattern for the right application. What I would say is that you should make your application consistent. Commented Jan 8, 2016 at 17:30
  • Maybe you could come up with a generic service that only forwards your request to the repository and then returns. This could be useful to keep a uniform interface and would be simple if in the future you need to add a real service to do something before call the repository. Commented Jan 8, 2016 at 19:13

4 Answers 4


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.

  • 3
    Well said! My preference is to call repositories, and only in cases, then a repository is not enough (i.e. two entities must be modified by using different repositories), I create a service which is responsible of this operation and call it from the controller.
    – Zygimantas
    Commented May 11, 2018 at 19:19
  • I had notice a rather complicated code just to justify the use of a service. Absurd, the least...
    – Gi1ber7
    Commented Jan 16, 2019 at 14:35
  • So my repository returns a list of 'business objects' that I need to convert into 'xml objects', is that reason enough to have a service layer? I'm calling a method on each object to convert it to another type and add to a new list.
    – mal
    Commented Oct 24, 2019 at 9:02
  • Direct DAO Access is dangerous in controllers, it can make you susceptible for SQL injections and gives access to dangerous actions like ,,deleteAll'' I would definitely avoid it.
    – Anirudh
    Commented Oct 28, 2019 at 15:05
  • @Anirudh Calling services that call DAO doesnt prevent SQL injections either. It could even be worse if you forgot your checks in one specific service. IMO it is best to sanitize/check inputs first in the controller flow, by callin a reusable service. Then when this step is handled, you can access DAO simply from controller, and if you need more logic, you can use services, pass DAO to them, and call DAO directly from them. I like to use controllers as a dispatcher calling services, dao, and views generators. Commented Dec 7, 2020 at 10:16

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);


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)
                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

                return RedirectToAction("Index", "Home");
                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:

  • 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:
    1. Manage the lifetime of a transaction, or
    2. Create a Unit Of Work object to manage the lifetime of a transaction
  • 4
    -1 for putting DbContext into a controller rather than a repo. The repo is meant to manage data providers so nobody else has to in the event a data provider changes (from MySQL to flat JSON files for instance, change in one place) Commented Jan 8, 2016 at 19:30
  • @JimmyHoffa: I'm actually looking back at the code I've written, and to be honest, I create a "context" object for my repositories, not necessary the database. I think DbContext is a bad name in this case. I'll change that. I use NHibernate, and the repositories (or context if it's handy) manage the database end of things, so changing persistence mechanisms doesn't require code changes outside the context. Commented Jan 8, 2016 at 19:36
  • you seem to be confusing controller with repository by the looks of your code... I mean, your "context" is all wrong and should absolutely not exist in the controller. Commented Jan 8, 2016 at 19:58
  • 7
    I don't have to answer and I'm not sure this is a good question to begin with, but I down vote because I think your approach is bad design. No hard feelings, I just discourage contexts being owned by controllers. IMO a controller shouldn't be starting and committing transactions like this. That's the job of any number of other places, I prefer controllers to delegate everything that's not simply fulfilling the HTTP request to somewhere else. Commented Jan 8, 2016 at 20:29
  • 1
    A repository, it's typically responsible for all data context information to ensure that nothing else needs to know anything about data concerns other than what the domain itself needs to know of Commented Jan 8, 2016 at 23:39

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:

  • Somehow you need to add/modify role access for the data. You have to add the logic in any controllers that call the repository, or you have to put it in the repository (will broke logic of other services that call this repo). If you call via a service, you just need to modify the service, or just add another service, no need to worry will broke other services that call that repo.
  • Let's say you have two clients of your application (Website and REST API), in the future, if you have to modify role access or business logic, you have to modify on both website and REST API controllers.
  • 9
    If the service does nothing else than calling a method on the repository, adding a Service is pretty useless.
    – Trace
    Commented Nov 29, 2021 at 19:19
  • I think it really depends on the project complexity as well as team. If the operations are basically just CRUD most likely just access the repo. More complex business logic would eventually demand a service and logic can be abstracted there. But perhaps your team is very junior and your app could scale. Even if it's currently just a simple CRUD app, I might still require a service layer even if its just a pass through to help describe the architecture and ensure SRP is highlighted. When the demand for it comes along, there's little to no confusion of where it should go.
    – GHOST-34
    Commented Dec 1, 2021 at 19:42
  • You should only write a service class if it does more just be a go between. Service should catch exceptions, deal with empty or null responses, get additional data that Repo requires, like call other repos for data so repo itself doesn't call other repos, etc. Controller shouldn't do heavy lifting but calling a Repo class is not heavy lifting
    – James
    Commented Oct 28, 2022 at 7:34
  • Simple services are not useless. They provide transactional control. I prefer to always call services with @Transactional anotation to prevent erratic Hibernate behaviours. For example, to prevent Hibernate to automatically persist entities that I read directly from DAOs.
    – Rober2D2
    Commented Jun 5, 2023 at 8:58

Not the answer you're looking for? Browse other questions tagged or ask your own question.