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?
2 Answers 2
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.
-
\$\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\$Professor of programming– Professor of programming2017年07月26日 15:29:42 +00:00Commented 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\$Amr Elgarhy– Amr Elgarhy2017年07月26日 15:31:23 +00:00Commented Jul 26, 2017 at 15:31
-
\$\begingroup\$ The
IPermissionService
is contained in the application layer. \$\endgroup\$Professor of programming– Professor of programming2017年07月26日 15:33:30 +00:00Commented 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\$Amr Elgarhy– Amr Elgarhy2017年07月26日 15:34:42 +00:00Commented 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\$Amr Elgarhy– Amr Elgarhy2017年07月26日 15:38:50 +00:00Commented Jul 26, 2017 at 15:38
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.