Is it a good idea to extract controller logic into a service that extends repository functionality

Question

Hi,

I had a controller POST action that is called List that accepts a status variable which can be the following values { "all", "active", "inactive}. Then I made repository calls based on the the value of "status" inside of the controller. The controller looked like this:

    [HttpPost]
    public ActionResult List(string status)
    {
        return View(GetJobTitlesByStatus(status));
    }

    private IList<JobTitle> GetJobTitlesByStatus(string status)
    {
        IList<JobTitle> jobTitles;

        switch (status)
        {
            case "All":
                jobTitles = jobTitleRepository.GetAll();
                break;
            case "Active":
                jobTitles = jobTitleRepository.GetActive();
                break;
            case "Inactive":
                jobTitles = jobTitleRepository.GetInactive();
                break;
            default:
                jobTitles = new List<JobTitle>();
                break;
        }
    }

I decided that the code in the switch statement was too much to be inside of a controller, so I extracted this out into a service and this service then makes the appropriate repository calls. For example:

public class JobTitleService : JobTitleRepository, IJobTitleService, IJobTitleRepository
{
    public JobTitleService(ISession session) : base(session) { }
    public IList<JobTitle> GetJobTitlesByStatus(string status)
    {
        IList<JobTitle> jobTitles;

        switch (status)
        {
            case "All":
                jobTitles = base.GetAll();
                break;
            case "Active":
                jobTitles = base.GetActive();
                break;
            case "Inactive":
                jobTitles = base.GetInactive();
                break;
            default:
                jobTitles = new List<JobTitle>();
                break;
        }

        return jobTitles;
    }
}

I personally think this works great, expecially since I'm using dependency injection to get the service into the controller. I have the following questions:

1) Do you think it is a good idea to extract the switch statement logic from the controller?
2) Do you think that having JobTitleService inherit from JobTitleRepository is better than having an IJobTitleRepository passed into the constructor of the Service (using dependency injection)?
3) Is there a special name for the Design Pattern used for the the JobTitleService?

Answer 1

1) Yes, business logic shouldn't be in the controller. Controllers really just connect the presentation to the logic. Ideally, the logic is performed in a more object-oriented approach than in the more procedural approach you have here, but don't be dissuaded by that. Procedural isn't a bad thing as long as the code is simple and supportable. What this design does for you is allow you to move your service class into an actual separate service if you ever need to.

2) I'm having trouble quantifying it, but I would be hesitant to have a service inherit from a repository. In this particular design I would view the services as the place to store business logic procedures and the repositories would be injected in as the persistence-aware components (the business logic ideally should not be persistence-aware). I just think it separates the concerns more cleanly and makes it easier to fully grok the design as the system grows. But this may just be my personal opinion.

3) I'm sure there's an official name that can be put on a resume somewhere :) "Service Oriented Architecture" comes to mind, along with other buzzwords. I've never been big on terminology, so I can't help much on this part. But there's plenty of reading material on the subject. This looks like a good read for that sort of thing: http://msdn.microsoft.com/en-us/library/ms954638.aspx Maybe look here for resources as well: http://www.soapatterns.org/

Answer 2

1. Yes - your controller should be responsible for selecting a result, preparing the model and then passing the model to the view for rendering. Everything else should be delegated somewhere else, typically services. Taken directly from wikipedia:

   The controller receives input and initiates a response by making calls on model objects. A controller accepts input from the user and instructs the model and viewport to perform actions based on that input.

2. No, your service should not inherit from the repository. The service needs to collaborate with the repository to do its function - thus it is a 'has-a' not an 'is-a' relationship. Right now this might not be obvious as your service is just a pass-through to the repository but as soon as your service begins to do what services are supposed to do (co-ordinate actions from one or more collaborators) it'll become immediately apparent as you will only be able to inherit from one of those collaborators.

   In fact, in this case if your service is only ever going to be a a direct wrapper around the repository it's not adding anything and is just a layer of indirection - if it stays that simple I would dispense with it and query the repository directly. Usually though things don't stay that simple for long though.

3. No, not really.