Is it good Practice Call Business Logic from ViewModel
Asked Answered
D

4

5

I am working on a large ASP.NET MVC Project(about 15 separate projects). We are using a Facade Design Pattern to call Business logic as well as other Projects.

Question: in MVC application is it a best practice to call a Facade from the ViewModel?

I am using single facade instances to call all the functions. I create a ViewModel for each Action and populate it with data from within the ViewModel. These results are making the ViewModel larger, but the Controller Action gets thinner because we are doing the work in the ViewModel now. In the ViewModel constructor I pass the facade instance and take what's needed from the business logic layer.

public class MyViewModel
{
    private Facade _Facade;
    public IEnumerable<SomeModel> Collection { get; set; }
    public IEnumerable<SelectListItem> Years { get; set; }
    public IEnumerable<SelectListItem> Quarters { get; set; }
    public int SelectedYear { get; set; }
    public int SelectedQuarter { get; set; }


     public BottomUpForecastViewModel(EXFacade facade)
    {
        this._Facade = facade;
        this.Years = GetFinancialYears();
        this.Quarters = GetFinancialQuarters();
        this.SelectedYear = DateTime.Now.Year;
        this.SelectedQuarter = TimePeriods.GetQuarterNoForDate(DateTime.Now);
        Collection = GetMonthlyCollection(SelectedYear, SelectedQuarter);// Take data     from the _Facade(call facade)

    }

}

  public class MyController : Controller
  {


    [AcceptVerbs(HttpVerbs.Get)]
    public ActionResult BottomUpForecast()
    {

        return View(new MyViewModel(facade));
    }

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult BottomUpForecast(MyViewModel model)
    {

        return View();

    }

}

Is this good practice?

Do you have a suggestion for a better approach taking into consideration that we don't need to worry about Dependencies?

UPDATE : I found an interesting article about how to make controllers Lean "Put them on a diet": http://lostechies.com/jimmybogard/2013/12/19/put-your-controllers-on-a-diet-posts-and-commands/**

Dodson answered 10/4, 2014 at 5:40 Comment(1)
You can refer to my answer to this post. I wouldn't populate your ViewModels in the controller. This increases the amount of code in the controller and I believe that is generally bad. Instead simply just call your Business Layer from your Controller and have the Business layer populate your VM. My Controller code would like this... MyViewModel vm = new BusinessLayer().BottomForecastVM(); return view(vm);#21597680Expiration
D
13

Your idea is correct. Perfectly acceptable to call business logic from View Model. I do it all the time.

Unfortunately, your current implementation is strongly coupled to concrete types. You could use a little abstract refactoring:

In stead, in your Business layer, create an Interface, IEXFacade to pass bind your object and pass to your ViewModel:

public interface IEXFacade
{
   public IEnumerable<SomeModel> GetMonthlyCollection(int SelectedYear, int SelectedQuarter);
   public IEnumerable<SelectListItem> GetFinancialYears();
   public IEnumerable<SelectListItem> GetFinancialQuarters();
   public int getSelectedYear();
   public int getSelectedQuarter(DateTime dateTime);
}

and your EXFacade definition would look something like:

public class EXFacade : IEXFacade
{
   private TimePeriods _timePeriods = new TimePeriods();

   public int getSelectedYear()
   {
       return DateTime.Now.Year;
   }

   public int getSelectedQuarter (DateTime dateTime)
   {
       return _timePeriods.GetQuarterNoForDate(dateTime);
   }


   public IEnumerable<SomeModel> GetMonthlyCollection()
   {
           ....
           return MonthlyCollection;
   }

   public IEnumerable<SelectListItem> GetFinancialYears();
   {
           ....
           return MonthlyCollection;
   }

   public IEnumerable<SelectListItem> GetFinancialQuarters();
   {
           ....
           return MonthlyCollection;
   }

}

Now your View Model would take IEXFacade and would be more tolerant of change

public class MyViewModel
{
     MyViewModel(IEXFacade facade)
     {
        Years = facade.GetFinancialYears();
        Quarters = facade.GetFinancialQuarters();
        SelectedYear = facade.getSelectedYear();
        SelectedQuarter = facade.getSelectedQuarter (DateTime.Now);
        Collection = facade.GetMonthlyCollection(SelectedYear, SelectedQuarter);
    }


    //Keeping the Facade Object seems extraneous (unless I'm missing something)
    //private Facade _Facade;
    public IEnumerable<SomeModel> Collection { get; set; }
    public IEnumerable<SelectListItem> Years { get; set; }
    public IEnumerable<SelectListItem> Quarters { get; set; }
    public int SelectedYear { get; set; }
    public int SelectedQuarter { get; set; }
}

The goal is to de-couple dependence on your specific implementation of your EXFacade class by passing an interface. Now your EXFacade methods logic can change without breaking your view model. As long as the Interface (properties and signatures) remain the same.


Conclusion:

I'm not partial to calling logic directly from my ViewModel, as opposed to my Controller. But, it's often more convenient as it saves a step. Conversely, logic injected directly into your model is less obvious than if you consolidate it into your controllers. But the argument about "Fat Controllers" vs "Fat Models" is pretty even sided and I don't think either side is more correct.

What's more important is to understand that the Facade Pattern is meant to be an interface between your "Chatty" Logic Layer and your Presentation Layer. For the sake of Abstraction and de-coupling, the pattern calls for an Interface. As soon as you abstract your Facade with an Interface you can further de-couple by using an IOC container like NInject to Inject your Facade into your controllers or Models.

I strongly recommend using the Dependency Injection pattern in a large project like this.

Debouchment answered 21/4, 2014 at 17:35 Comment(9)
@unique, I hope I helped. I updated my answer a touch. I believe you are on the right track but would be helped by using an IOC container to implement Dependency Injection ;)Debouchment
Also important to notice is that in this way of implementing logic in your viewmodels, is to add the default constructor to the viewmodel also. If you don't add the default constructor you get issues with the ASP.NET MVC binder on a POST.Underlayer
Hello @DaveAlperovich and thanks for your reply. You mentioned about using DI, and I can entirely see how that is useful for the viewmodels, but of course the way that controllers initialize VM's right now means that given the current sample we would have to inject the facade into the controller and then pass it down to the VM's. Do you think it could be property injected instead during the constructor of the VM? What's your recommended way to go about using DI for VM's exactly?Intort
Specifically I was thinking about the case where we use Html.Partial a lot, and it seems we can only pass a single object down to the partial, which means we couldn't pass the facade any further down. So If VM's could instantiate the service they needed by themselves, then does it make sense to property inject the needed service into the constructor of VM's?Intort
@Worthy7, I wasn't imagining using DI for VM's. I was thinking of VM's making calls to services. The services would be managed by DI. Look at my examples: The VM has a service property which implements the Facade interface. Not sure if I explained that well.Debouchment
I see your approach, just means injecting the service into the controller first, then passing the instantiated service through to the model. Interesting approach. I have made "View Orchestrators" which build certain complex views for me using various services and repositories.Intort
Putting business logic in the viewmodel is actually a great way of doing it. The viewmodel is responsible for its own state in this way. I would like to dev my software like this from now on. The only thing I ask myself is how the viewmodel would access user input (say: pagesize & pageindex for some type of search result). I'm guessing you'd have to pass those into the viewmodel constructor along with the facade, from your mvc controller. Or is there another best practice for creating/returning the viewmodel to the view?Taphole
It just occurred to me that my current viewmodel creation method is an async method. It kind of has to be, because it calls async methods itself. If I were to move this business logic to a viewmodel's constructor, then I can't make use async/await anymore. Constructors can't be async. That's kind of a showstopper. To find the golden mean, I am now considering building an object whose sole purpose is to create viewmodels. It will get all my facades injected and it can use those to create viewmodels that require info from multiple facades. Anybody got a better suggestion? I'm all ears.Taphole
I should probably just give my viewmodels an async method that will let the viewmodel construct an instance of itself.Taphole
M
9

You'll be breaking the MVC pattern if you put business logic in the ViewModel. It's controller's job to construct the view, not the view constructing itself by receiving depenencies.

The ViewModel should be ignorant of other layers(View and Controller), thereby promoting loosely coupled architecture.

If your ViewModel becomes too large, you can create helper methods or classes just for constructing the ViewModel.

public class MyController : Controller
{
    [AcceptVerbs(HttpVerbs.Get)]
    public ActionResult BottomUpForecast()
    {
        return View(this.GetMyViewModel());
    }


    private MyViewModel GetMyViewModel()
    {
        var viewModel = new MyViewModel()
        {
            Years = this.facade.GetFinancialYears();
            Quarters = this.facade.GetFinancialQuarters();
            SelectedYear = DateTime.Now.Year;
            SelectedQuarter = this.facade.TimePeriods.GetQuarterNoForDate(DateTime.Now);
            Collection = this.facade.GetMonthlyCollection(SelectedYear, SelectedQuarter);
        }

        return viewModel;
    }
}

// Thin ViewModel
public class MyViewModel
{
    public IEnumerable<SomeModel> Collection { get; set; }
    public IEnumerable<SelectListItem> Years { get; set; }
    public IEnumerable<SelectListItem> Quarters { get; set; }
    public int SelectedYear { get; set; }
    public int SelectedQuarter { get; set; }
}

Interesting discussion about this topic here: https://mcmap.net/q/874299/-fetching-data-within-an-asp-net-mvc-viewmodel-class

Mavilia answered 11/4, 2014 at 5:23 Comment(3)
Thank you very much for the comment. Then what is the Use of Models in ASP.NET MVC Application?Dodson
@Dodson - Most of the time, they are the same. The key difference is that a model represents a domain entity, a ViewModel is tailored for a specific view containing dropdowns, etc. IE: PersonModel vs EditPersonViewModelMavilia
Continuing with Yorro's answer. You have 3 tiers right, data biz and presentation. Data layer will call databases and populate "Domain Entities", Business will simply transfer the Domain Entity into a ViewModel. The Business layer will simply pass the ViewModel to the Controller without having the Controller deal with any of the populationExpiration
F
8

The view model is a model for the view. It should contain data (model) and any logic required to move that data into and out of the view. It shouldn't know anything about any other layers. It shouldn't even depend on the controller, never mind anything below that. It's the job of the controller to populate that view model so the job of the controller to invoke the business logic.

Fishbolt answered 10/4, 2014 at 5:54 Comment(5)
But in this context to populate the view model from the controller action result view large controller action isn't it?Dodson
Absolutely, but that is the controller's job. By pushing that logic into the view model you create an extra dependency.Fishbolt
The ViewModel is a cross-cutting layer that must act as a "Data Transfer Object". If you put access to Business Logic inside of your ViewModel you'll break the Single Responsability Principle by adding extra work to this layer.Merilee
@Merilee Followed the pattern you outlined in aspnet/mvc app. An ViewModel is not a DTO. In retrospect I would rather call ViewModel.AddPost() than than businessLayer.Posts.Add(post). Are there any realistic drawbacks to adding behavior(delegating to the business layer) to the ViewModel(assuming there is no logic in it)? Remember MVVM is not MVC and ViewModels might be responsible for many unrelated DTOs! In effect this would be a statement that the ViewModel is really just another layer which could clean up the Controller a bit.Heddi
@Heddi Calling the business layer from the ViewModel creates increases the coupling of the software. I agree that MVVM is not MVC, but using MVVM with ASP.NET MVC framework, you have no escape ;). The controller should be responsible to translate data from the business layer to the viewmodel (I recommend using the AutoMapper for this dirty job).Merilee
S
1

Just to add to @Yorro post, MVVM actually uses this pattern where VM is responsible for all such activities. It will be preferred to use Controller for such actions in MVC.

Spevek answered 12/7, 2018 at 13:32 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.