1
\$\begingroup\$

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?

\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Which layer would the service be contained in then as it would only really apply to the presentation layer, but I thought services should be part of the application layer? \$\endgroup\$ – Matthew Bonner Jul 26 '17 at 15:29
  • \$\begingroup\$ Is your Service just exist on the presentation layer, so this mapper class should be in the presentation layer as well \$\endgroup\$ – Amr Elgarhy Jul 26 '17 at 15:31
  • \$\begingroup\$ The IPermissionService is contained in the application layer. \$\endgroup\$ – Matthew Bonner Jul 26 '17 at 15:33
  • 1
    \$\begingroup\$ and what about the ViewModels? if they are in the presentation layer so the mapper will be there as well. \$\endgroup\$ – Amr Elgarhy Jul 26 '17 at 15:34
  • 1
    \$\begingroup\$ hmm, naming is your call, but it can be called ViewModelsBuilder ViewModelProcessor, something which construct view models. \$\endgroup\$ – Amr Elgarhy Jul 26 '17 at 15:38
1
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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