views:

254

answers:

2

I edited my whole question, so do not wonder :)

Well, I want to have an ActionResult that takes domain model data and some additional parameters, i.e page index and page size for paging a list. It decide itself if it returns a PartialViewResult or a ViewResult depending on the kind of web request (ajax request or not).

The reffered data shall be mapped automatically by using an IMappingService, which is responsible for transforming any domain model data into a view model. The MappingService uses AutoMapper for simplicity.

MappingActionResult:

public abstract class MappingActionResult : ActionResult
{
    public static IMappingService MappingService;
}

BaseHybridViewResult:

public abstract class BaseHybridViewResult : MappingActionResult
{
    public const string defaultViewName = "Grid";

    public string ViewNameForAjaxRequest { get; set; }
    public object ViewModel { get; set; }

    public override void ExecuteResult(ControllerContext context)
    {
        if (context == null) throw new ArgumentNullException("context");
        var usePartial = ShouldUsePartial(context);
        ActionResult res = GetInnerViewResult(usePartial);

        res.ExecuteResult(context);
    }

    private ActionResult GetInnerViewResult(bool usePartial)
    {
        ViewDataDictionary viewDataDictionary = new ViewDataDictionary(ViewModel);
        if (String.IsNullOrEmpty(ViewNameForAjaxRequest))
        {
            ViewNameForAjaxRequest = defaultViewName;
        }

        if (usePartial)
        {
            return new PartialViewResult { ViewData = viewDataDictionary, ViewName = ViewNameForAjaxRequest };
        }

        return new ViewResult { ViewData = viewDataDictionary };
    }

    private static bool ShouldUsePartial(ControllerContext context)
    {
        return context.HttpContext.Request.IsAjaxRequest();
    }
}

AutoMappedHybridViewResult:

public class AutoMappedHybridViewResult<TSourceElement, TDestinationElement> : BaseHybridViewResult
{
    public AutoMappedHybridViewResult(PagedList<TSourceElement> pagedList)
    {
        ViewModel = MappingService.MapToViewModelPagedList<TSourceElement, TDestinationElement>(pagedList);
    }

    public AutoMappedHybridViewResult(PagedList<TSourceElement> pagedList, string viewNameForAjaxRequest)
    {
        ViewNameForAjaxRequest = viewNameForAjaxRequest;
        ViewModel = MappingService.MapToViewModelPagedList<TSourceElement, TDestinationElement>(pagedList);
    }

    public AutoMappedHybridViewResult(TSourceElement model)
    {
        ViewModel = MappingService.Map<TSourceElement, TDestinationElement>(model);
    }

    public AutoMappedHybridViewResult(TSourceElement model, string viewNameForAjaxRequest)
    {
        ViewNameForAjaxRequest = viewNameForAjaxRequest;
        ViewModel = MappingService.Map<TSourceElement, TDestinationElement>(model);
    }
}

Usage in controller:

public ActionResult Index(int page = 1)
{
    return new AutoMappedHybridViewResult<TeamEmployee, TeamEmployeeForm>(_teamEmployeeRepository.GetPagedEmployees(page, PageSize));
}

So as you can see the IMappingService is hidden. The controller should not know anything about the IMappingService interface, when AutoMappedHybridViewResult is used.

Is the MappingActionResult with the static IMappingServer appropriate or am I violating the DI principle?

+1  A: 

I think a better design is to have a ViewResultFactory that depends on IMappingService, then you can inject that into your controller. Then you call it like so:

public class MyController : Controller
{
    IViewResultFactory _viewResultFactory;
    ITeamEmployeeRepository _teamEmployeeRepository;

    public MyController(IViewResultFactory viewResultFactory)
    {
        _viewResultFactory = viewResultFactory;
    }

    public ActionResult MyAction(int page, int pageSize)
    {
        return
            _viewResultFactory.GetResult<TeamEmployee, TeamEmployeeForm>(
                _teamEmployeeRepository.GetPagedEmployees(page, pageSize));
    }
}

The implementation would like this (you would need to create overloads for each of your HybridViewResult constructors):

public HybridViewResult<TSourceElement, TDestinationElement> GetResult<TSourceElement, TDestinationElement>(PagedList<TSourceElement> pagedList)
{
    return new HybridViewResult<TSourceElement, TDestinationElement>(_mappingService, pagedList);
}

That way you hide the implementation from your controllers, and you don't have to depend on the container.

Robin Clowers
I have updated my question
Rookian
what does your factory return? A PagedList<TeamEmployeeForm> or the (Partial-)ViewResult?
Rookian
I updated my answer, it would return the view result
Robin Clowers
How would the injections looks like? As I wrote already, I think injecting IMappingService in the controller makes no sense, because the controller is not depending on the service, but the ViewResult is.
Rookian
Your base controller would take a ViewResultFactory instance in the constructor, and the factory would take an IMappingService instance. Like you said, the controller does not know anything about the mapping service, its only job is to create view results.
Robin Clowers
How would your base controller and derived controller look like?
Rookian
I added a complete controller example, the base controller does not have to do anything special for this to work.
Robin Clowers
Ok, now I understand you. I will try your approach ...
Rookian
Cool, let me know what you think. Don't forget to mark my answer as accepted if you end up using this approach :)
Robin Clowers
It looks nice. But I did not used an interface for the factory, because the factory is located in the UI layer.
Rookian
A: 

There are a few different points that you could inject IMappingService. http://codeclimber.net.nz/archive/2009/04/08/13-asp.net-mvc-extensibility-points-you-have-to-know.aspx is a good site for help in picking the appropriate extensibility points for .NET MVC.

If you want to stick with having this functionality be a derived ActionResult, then I think you could put the dependency in the ActionInvoker if you want to, but the Controller makes more sense to me. If you don't want the IMappingService in the Controller, you could always wrap it in a HybridViewResultFactory, and access that object in the Controller. In that case your shortcut methods would look like:

public HybridViewResult<TSourceElement, TDestinationElement> AutoMappedHybridView<TSourceElement,TDestinationElement>(PagedList<TSourceElement> pagedList, string viewNameForAjaxRequest)
{
    HybridViewResultFactory.Create<TSourceElement, TDestinationElement>(pagedList, viewNameForAjaxRequest);
 }

etc.

I'm not sure why you need to use an ActionResult, but if there is no reason that makes it explicitly necessary, you could create a HybridViewModel class and a HybridViewModelBinder class that is injected with the mapping service dependency.

I am assuming you want to use constructor injection, but if you have the StructureMap dependency in your UI assembly, you could access a static dependency resolver class (like Clowers said).

This question would be easier to give a definite answer to if I understood why you using an ActionResult.

It seems like you are using the action result to handle two functionalities that do not necessarily go together all the time, and that could be used separately. Also, there is not a clear indication that it needs to be in an ActionResult.

Presumably, you could (a) leverage the Automapper functionality for results other than html (ViewResult) output, and (b) you could leverage the functionality of auto-detecting ajax requests without needing to automap the model.

It seems to me like the automapping of the view model could be used to inject the view model into the controller action directly, thus removing the controller's dependency on the IMappingService. What you would need is a ModelBinder class to be injected with your IMappingService (the implementation of which I assume contains a repository or datastore type dependency).

Here is a good article explaining how to leverage model binders: http://odetocode.com/blogs/scott/archive/2009/04/27/6-tips-for-asp-net-mvc-model-binding.aspx.

Then you can overwrite the DefaultModelBinder in the classes that need to be Automapped as follows:

   public ActionResult DoItLikeThis([AutoMap(typeof(MyDomainModelClass))]MyViewModelClass viewModel){
               //controller action logic
   } 

Now, regarding the HybridViewResult, I would suggest that you handle this with an Action Filter instead. So, you could just use ActionResult or ViewResultBase as the Result type of your action method and decorate it with an action filter, i.e.:

   [AutoSelectViewResult]
   public ViewResultBase AndDoThisLikeSo(){
               //controller action logic
   } 

I think overall this will be a much better solution than coupling these two functionalities to an ActionResult.

smartcaveman
The "hybrid" in the class "HybridViewResult" stands for automatically detecting if there is an ajax request or not. "HybridViewResult" returns a PartialViewResult if it is an ajax request otherwise it returns a ViewResult. I hope this makes sense :)
Rookian
In that case, I would suggest ditching the 'AutoMappedHybridView' all together, and use an AutoMapActionFilter and a AutoSelectResultTypeActionFilter on the methods that require each functionality. My reasoning? ... this is getting long I'm going to append the actual post so read above...
smartcaveman
I am scared to have so much ActionFilters on my controller actions.
Rookian
What you could do is use a controller base class, with shortcut methods decorated with the Action Filters, (like how View() is a short cut for view result). So, you could use AutoSelectView() in the subcontrollers, and in the base controller you could have [AutoSelectViewResult]ViewResultBase AutoSelectView(Model){//logic} in your base class.You could also avoid the Model Binder attribute by doing something like the following in your global.asax:ModelBinders.Binders.DefaultBinder = AutoMapBinder; This hides the attributes but maintains the extensibility.
smartcaveman