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?

asked Jul 26, 2017 at 15:04
\$\endgroup\$

2 Answers 2

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.

alecxe
17.5k8 gold badges52 silver badges93 bronze badges
answered Jul 26, 2017 at 15:27
\$\endgroup\$
6
  • \$\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\$ Commented Jul 26, 2017 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\$ Commented Jul 26, 2017 at 15:31
  • \$\begingroup\$ The IPermissionService is contained in the application layer. \$\endgroup\$ Commented Jul 26, 2017 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\$ Commented Jul 26, 2017 at 15:34
  • 1
    \$\begingroup\$ hmm, naming is your call, but it can be called ViewModelsBuilder ViewModelProcessor, something which construct view models. \$\endgroup\$ Commented Jul 26, 2017 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.

answered Jul 26, 2017 at 18:58
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.