MVC - data calculations best practice - viewmodel vs. controller
Asked Answered
S

3

5

rI need some advice on where to run a calculation on data.

I have a viewmodel that contains all the fields that I need for my calculation and I created the following for one of my calculations:

public class CommissionVM
{
public int? LoanAmountLock { get; set; } // from loan table    
public decimal BranchRev { get; set; }   // from revenue table
public decimal BranchProcessFee { get; set; }   // from revenue table

public decimal BranchGrossTotal
    {
        get
        {               

            return Convert.ToDecimal(LoanAmountLock * (BranchRev/ 100) + BranchProcessFee);
        }
    }

}

I tried to use the Model.BranchGrossTotal in my view, but it is returning 0. I think I have an order-of-operations problem. The values LoanAmountLock, BranchRev, and BranchProcessFee are returned as the results of a query:

public ActionResult Reconcile(int? id, string RevenueLoanType)
    {
var model = new CommissionVM()
        {

            Loan = db.Loan.FirstOrDefault(a => a.id == id ),                
            BranchRevenues = db.BranchRevenues.FirstOrDefault(a => a.RevenueLoanType == RevenueLoanType),  
        };
return View(model);
}

I originally was able to get these calculations to work by doing all the math in the controller after I populate the viewmodel with the query, but there will be about 10 calculations, and from what I understand, I shouldn't clutter up my controller with business logic.

What is the best solution for this? Do I need to create another class for the calculations? If so, how do I populate that class with my data and use it in my controller?

EDIT: I am not sure how to set up the business classes and use them in the controller. Can anyone point me in the direction of a tutorial?

Shrivel answered 30/7, 2015 at 17:45 Comment(2)
check this link, it might help you for your Business Logic design: #1016313Lumberman
I don't like calculations on my view models -- you can't reuse the calculation easily elsewhere and it is harder to test and debug. Create a separate classes to do business logic -- Luis' answer has a good example.Soilure
S
3

I don't like calculations on my view models -- you can't reuse the calculation easily elsewhere and it is harder to test and debug. Create separate classes to do business logic.

Your business logic classes can either return your view models or return values you use to populate those models. The trade-off is ease of use with reusability.

I generally favor returning the value rather than a big object so my services are more reusable.

Controller

public class BranchController : Controller
{
    private IBusinessService service;

    public BranchController()
    {
        this.service = new BusinessService(db);
    }

    public ActionResult Reconcile(int? id, string RevenueLoanType)
    {
        var model = new CommissionVM
        {
            BranchGrossTotal = this.service.GetBranchGrossTotal(id, RevenueLoanType),
            ...
        };

        return View(model);
    }
}

Service

You can make any number of these and your controllers would use them as needed. If you need a query you should pass the DbContext instance or you may have problems with related entities on separate contexts.

public interface IBusinessService
{
    decimal GetBranchGrossTotal(int id, string revenueLoanType);
}

public class BusinessService : IBusinessService
{
    private DbContext db;

    public BusinessService(DbContext db)
    {
        this.db = db;
    }

    public decimal GetBranchGrossTotal(int id, string revenueLoanType)
    {
        var branch = db.Branch.First(b => b.Id == id);
        // do stuff
        return total;
    }
}

You could fully-populate and return a view model in your GetBranchGrossTotal() if you choose.

Soilure answered 30/7, 2015 at 20:21 Comment(4)
Thank you for your specific answer. I am trying to get this to work for me. What does the IBusinessService refer to? I am seeing an "Are you missing a directive or an assembly reference) error.Shrivel
Make interfaces for your services now as it's low effort at this point. It's entirely optional now but eventually you may want to do Dependency Injection and it would be very easy to do since you have already defined interfaces for your services. You may also later find a repeated pattern for your services and you'll end up relying interfaces in your controllers.Soilure
Another question - My DBContext is named MWFData. To set the db context in the service, what do I need to use. Private MWFData db. Then I used this.db = MWFData; I am seeing this error: 'System.Data.Entity.DbContext' is a 'type' but is used like a 'variable'Shrivel
Yes, use whatever name your DbContext class uses. In the service constructor set the private db to the passed MWFData instance from the controller.Soilure
S
7

You should not do the calculation in your controller nor in your view model. You should do it in the Business Layer. Think about View Models are really simple classes that contain data to be displayed to the user, that's it.

Regarding the calculation, you should convert one of the terms to decimal, not the result of the calculation. If you divide integers, you get an integer.

You could create a class and call it CommissionService for example. That class should call your Data Layer to get the data, do any extra calculation and return the data (maybe a DTO) to the controller. The controller should create View Models based on the DTO and send them to the view.

enter image description here

Read these articles:

1) https://msdn.microsoft.com/en-us/library/hh404093.aspx

2) http://www.asp.net/mvc/overview/older-versions-1/models-%28data%29/validating-with-a-service-layer-cs

3) http://blog.diatomenterprises.com/asp-net-mvc-business-logic-as-a-separate-layer/

4) http://sampathloku.blogspot.com.ar/2012/10/how-to-use-viewmodel-with-aspnet-mvc.html

Sting answered 30/7, 2015 at 17:56 Comment(4)
At this point, I don't have a Business Layer. Can you show me an example of how to set that up and then call it in my controller? I am a bit of a newbie...Shrivel
You could create a class and call it CommissionService for example. That class should call your Data Layer to get the data, do any extra calculation and return the data (maybe a DTO) to the controller. The controller should create View Models based on the DTO and send them to the view.Sting
I am not sure how to set up the business classes and use them in the controller. Can anyone point me in the direction of a tutorial?Shrivel
I have updated my answer providing an imagen (it's taken from link 1) that divided the app into layers and describes what you should have on each layer. I also provided a series of links that are gonna help you understand all these concepts.Sting
S
3

I don't like calculations on my view models -- you can't reuse the calculation easily elsewhere and it is harder to test and debug. Create separate classes to do business logic.

Your business logic classes can either return your view models or return values you use to populate those models. The trade-off is ease of use with reusability.

I generally favor returning the value rather than a big object so my services are more reusable.

Controller

public class BranchController : Controller
{
    private IBusinessService service;

    public BranchController()
    {
        this.service = new BusinessService(db);
    }

    public ActionResult Reconcile(int? id, string RevenueLoanType)
    {
        var model = new CommissionVM
        {
            BranchGrossTotal = this.service.GetBranchGrossTotal(id, RevenueLoanType),
            ...
        };

        return View(model);
    }
}

Service

You can make any number of these and your controllers would use them as needed. If you need a query you should pass the DbContext instance or you may have problems with related entities on separate contexts.

public interface IBusinessService
{
    decimal GetBranchGrossTotal(int id, string revenueLoanType);
}

public class BusinessService : IBusinessService
{
    private DbContext db;

    public BusinessService(DbContext db)
    {
        this.db = db;
    }

    public decimal GetBranchGrossTotal(int id, string revenueLoanType)
    {
        var branch = db.Branch.First(b => b.Id == id);
        // do stuff
        return total;
    }
}

You could fully-populate and return a view model in your GetBranchGrossTotal() if you choose.

Soilure answered 30/7, 2015 at 20:21 Comment(4)
Thank you for your specific answer. I am trying to get this to work for me. What does the IBusinessService refer to? I am seeing an "Are you missing a directive or an assembly reference) error.Shrivel
Make interfaces for your services now as it's low effort at this point. It's entirely optional now but eventually you may want to do Dependency Injection and it would be very easy to do since you have already defined interfaces for your services. You may also later find a repeated pattern for your services and you'll end up relying interfaces in your controllers.Soilure
Another question - My DBContext is named MWFData. To set the db context in the service, what do I need to use. Private MWFData db. Then I used this.db = MWFData; I am seeing this error: 'System.Data.Entity.DbContext' is a 'type' but is used like a 'variable'Shrivel
Yes, use whatever name your DbContext class uses. In the service constructor set the private db to the passed MWFData instance from the controller.Soilure
H
2

First of all, the properties you are assigning to your CommissionVM on your controller do not match the ones declared on your model. You assign Loan and BranchRevenues, when you have only LoanAmountLock and BranchRevs available on your model. Please notice that the Loan property is an object itself, and the LoanAmountLock must be retrieved from this object (Loan.LoanAmountLock). The same happens with the BranchRevenues object. You should assign the BranchRevs to the respective property of the BranchRevenues object as needed. If you do not do this, then the values will default to 0 and when trying to calculate the BranchGrossTotal it will obviously be 0.

Another reason, assuming that you are correctly populating your model properties, is that the FirstOrDefault method, renders null values because there is no such entity. This will result also in the BranchGrossTotal to be 0.

You are right that you do not need to clutter your controller neither with calculations nor with db access. I would create a business class ComissionBusiness and instantiate it at the top of your controller. This class would have a method which performs all calculations. You should move the Reconcile method to your new business class method and call it on the reconcile action. Something like (excuse the lack of syntax)

public MyController : Controller {

public ComissionBusiness comissionBusiness;

public MyController(){
comissionBusiness  = new ComissionBusiness();
}

public ActionResult Reconcile(int? id, string RevenueLoanType)
    {
var model = comissionBusiness.Reconcile(id, revenueLoanType);
return View(model);
}
}
Hussy answered 30/7, 2015 at 18:0 Comment(4)
This is a newbiew question, but where does the query and viewmodel CommisionVM come into play then? I have been doing everything in the controller up to this point, so I am not sure how to set it up.Shrivel
@Shrivel Pass the dbContext instance to the business class' constructor. Do the queries in the business functions then return CommissionVM or have it return a value and you populate VM properties in the controller.Soilure
@Shrivel what Jasen said is true. The business class should return a ComissionVM object when called. The business class will be in charge of querying the db and populating your view model.Hussy
A Business class should never return a VM. VM means View Model so it is related to the view. Business classes don't care about views. The controller could be the one that transforms a business object or DTO into a VM.Sting

© 2022 - 2024 — McMap. All rights reserved.