Pass service to view model or leave thin controller as is

Question

I have the following action:

    public PartialViewResult Analyze()
    {
        var viewModel = new WelcomeAnalyzeViewModel
        {
            IsDrugsPanelVisible = !Service.IsMDStandalone(),
            IsDevicesPanelVisible = Service.IsSubscribedToMedTech(),
            IsCompaniesPanelVisible = !Service.IsCTStandalone(),
            IsCountriesPanelVisible = !Service.IsCTStandalone() || (Service.IsCTStandalone() && Service.HasImsAddOn()),
            IsFinancePanelVisible = !Service.IsCTStandalone()
        };

        return PartialView("Analyze", viewModel);
    }

I'm not sure if I should just pass the Service property to the constructor of the WelcomeAnalyzeViewModel or leave this code as is but from the perspective of a thin controller this doesn't feel right.

The Service property contains an object to a permission service (IPermissionService) which can be used for other applications outside of the web environment.

Would it be acceptable for one service to call another service, so for instance I create an AnalyzeService which contains methods with calls to IPermissionService methods?

Answer 1

I would suggest having a separate class for doing this processing and mapping.
It will be a service or mapper class takes your Service object in the constructor and has a method which return WelcomeAnalyzeViewModel object.

Also you can do it all through the method, by passing your Service object to this method and return the WelcomeAnalyzeViewModel, this will make your controller very thin something like that:

public PartialViewResult Analyze()
{
        var viewModel = Mapper.GetWelcomeAnalyzeViewModel(Service);
        return PartialView("Analyze", viewModel);
}

public class Mapper
{
  public WelcomeAnalyzeViewModel(ServiceType service)
  {
      return new WelcomeAnalyzeViewModel
        {
            IsDrugsPanelVisible = !Service.IsMDStandalone(),
            IsDevicesPanelVisible = Service.IsSubscribedToMedTech(),
            IsCompaniesPanelVisible = !Service.IsCTStandalone(),
            IsCountriesPanelVisible = !Service.IsCTStandalone() || (Service.IsCTStandalone() && Service.HasImsAddOn()),
            IsFinancePanelVisible = !Service.IsCTStandalone()
        };
  }
}

And i will put this mapper class in the ViewModels project if you have a separate project for them.

Answer 2

I understand the point of having thin controllers, but I feel like this belongs in the controller.

Imagine if your ViewModels were in a separate project than your controllers and your services as well. Would you want to create a dependance between the ViewModels DLL and the Services DLL? I don't think so.

The view models are basically DTO, they shouldn't have access to your services.

Though, I don't know if your service calls are expensive, but since you repeat some calls more than once, you might want to keep them in a variable. If they are expensive, you'll get a much better performance, if they ain't well... You'll get... less calls to your service!

public PartialViewResult Analyze()
{
    bool isCTStandAlone = Service.IsCTStandalone();
    var viewModel = new WelcomeAnalyzeViewModel
    {
        IsDrugsPanelVisible = !Service.IsMDStandalone(),
        IsDevicesPanelVisible = Service.IsSubscribedToMedTech(),
        IsCompaniesPanelVisible = !isCTStandAlone,
        IsCountriesPanelVisible = !isCTStandAlone || (isCTStandAlone && Service.HasImsAddOn()),
        IsFinancePanelVisible = !isCTStandAlone()
    };

    return PartialView("Analyze", viewModel);
}

I think it's a dangerous path to try and "thin" everything. At some point, some of your classes will hold more code than others. You want your controllers to be as thin as possible and that's good. That's why you have services, view models, etc. But building these things belong in the controller sometimes.