Is passing a service reference to another service layer bad practice?

Question

I've got a C# MVC application that I break up in the following way: View -> Controller -> Service -> Repository

I use the thin controller practice with each view having a unique view model that's returned from the relevant service.

Quick Example: View: /NewAppointment/Step1

Its controler would look like this:

public ActionResult Step1()
{
  return View(_appointmentService.Step1GetModel() );
}

And the appointment service layer would look like this:

public Step1Model Step1GetModel()
{
  return new Step1Model();
}

Thus I have several different service layers in use through out my application, each implementing a distinct interface.

My question comes in when I need to have one service layer interact with another service layer. In this case, is it better practice to pass an interface reference to the service call, or should I let the controller handle collecting all the data and then pass the relevant results back to the service?

Example:

Say I want to populate my view model with the customer's information by default. The two ways I see of doing this are:

Pass an customer interface reference to the appointment service, and then let the appointment service call the appropriate GetCustomer method in the customer service...

In Code:

 private ICustomerService _customerService;
 private IAppointmentService _appointmentService;

 public ActionResult Step1()
 {
   var viewModel = _appointmentService.Step1GetModel( _customerService );
   return View(viewModel);
 }

OR

Let the controller handle the logic of getting the customer, then pass that result to the appointment service.

In Code:

private ICustomerService _customerService;
private IAppointmentService _appointmentService;

public ActionResult Step1()
{
    var customer = _customerService.GetCustomer();
    var viewModel = _appointmentService.Step1GetModel( customer );
    return View(viewModel);
}

I'm torn as to which would be better practice. The first keeps the controller nice and thin, but creates an inter-service dependency between the appointment service and the customer service. The second puts more logic into the controller, but keeps the services totally independent.

Anyone have thoughts as to which would be better practice?

Thanks~

Answer 1

Thinking about his purely conceptually I don't think it makes sense for your services to know anything about your view models. One of the main reasons to have a controller in the first place is to separate out your view logic from your business logic, but if your services are returning view specific data then they're inherently tied to your business logic.

Ideally I'd expect the method to look like this:

public ActionResult Step1()
{
    var customer = _customerService.GetCustomer();
    var appointment = _appointmentService.GetAppointmentFor(customer);

    var viewModel = new Step1ViewModel(customer, appointment);

    return View(viewModel);
}

To answer your question more directly though, I think it's fine for your services to know about each other, they're part of the same conceptually layer.

Also, one more thing...

It sounds like you have a lot of parallel class hierarchies what with having services, repositories and controllers. It might make more sense to use something like the unit of work pattern and a powerful ORM to do something like:

public MyController(IUnitOfWork unitOfWork)...

public ActionResult Step1()
{
    var customer = unitOfWork.Find<Customer>();
    var viewModel = new Step1ViewModel(customer.Appointment);
    return View(viewModel);
}

After all, the value of your application is in the model, not in the services.