I would like to suggest a potential refactoring of the ProductController.
The ProductController currently contains a method named PrepareProductDetailsPageModel(...) used to convert a Product EF model to a ProductDetailsModel view model ready for a ProductTemplate view.
The responsibility of the controller is to receive requests, handle the activities based on the request parameters, delegate the tasks to be performed and handle the next view to be shown. The responsiblity of this PrepareProductDetailsPageModel is to convert a model to a view model. Therefore, I would argue that this responsibility should be moved outside of the ProductController.
Potentially, this method could be moved to a (lets call it) ProductDetailsModelFactory say:
public class ProductDetailsModelFactory : IProductDetailsModelFactory
{
protected virtual ProductDetailsModel PrepareProductDetailsPageModel(Product product, ShoppingCartItem updatecartitem = null, bool isAssociatedProduct = false)
{
// existing code here
}
}
As the existing code is approximately 700 lines long, this would greatly simplify the controller (potentially halving the size of it). Additionally, the following dependencies (constructor injected) could potenitally be removed from the controller as they would instead be required by the ProductDetailsModelFactory.
manufacturerService
vendorService
productTemplateService
productAttributeService
productTagService
downloadService
productAttributeParser
shippingService
vendorSettings
seoSettings
Going one step further, the existing (approx) 700 line block of code for PrepareProductDetailsPageModel could be broken down further into methods within the ProductDetailsModelFactory. For example:
public class ProductDetailsModelFactory : IProductDetailsModelFactory
{
protected virtual ProductDetailsModel PrepareProductDetailsPageModel(Product product, ShoppingCartItem updatecartitem = null, bool isAssociatedProduct = false)
{
// simplified code
var somePictureModel = GetPictureModel();
//use somePictureModel to populate ProductDetailsModel
}
protected virtual SomePictureModel GetPictureModel()
{
//existing picture code to get pictures
}
//other methods
}
This would greatly simplify the PrepareProductDetailsPageModel method and furthermore, would allow developers to override these routines. If for example, a developer wanted to modify the way that pictures were retrieved (as I did before writing this post), I believe they would find it very difficult with the currently solution. With the proposal above, a developer could inherit from ProductDetailsModelFactory, override the GetPictureModel() method and alter as required and set up their new factory to be injected into the controller instead.
I would be interested to know what other developers who have more experience with nopCommerce think about this and to hear some of the disadvantages that I may not have considered.