Inefficient MVC ViewModel Making Multiple Calls to the Database?

Question

I clearly don't know what I'm doing. This MVC stuff is really blowing my mind in trying to keep with the pattern. I've been following the MVC tutorials as well as mega-googling and this is the corner I've painted myself into.

I have multiple similar pieces of data I'm trying to get to a view. I'm able to get my code to work, but to me it just looks like it's going to be highly inefficient as we start pulling large recordsets from the db due to multiple calls to the db. So, I have a OrderSummary class, inside the class is this:

public IEnumerable<Order> GetOrders()
{
   var orders = (from s in db.Orders
                 where s.UserId == uId
                 select s);

   return orders.ToList();
}

Then this:

public decimal GetGrossProfitTotal()
{
   var orders = (from s in db.Orders
                 where s.UserId == uId
                 select s);
   decimal? grossprofittotal = orders.Sum(s => s.Profit);

   return grossprofittotal ?? decimal.Zero;
}

So, if we take that last chunk of code and copy it for totalcommission and netprofittotal that's basically how I have things layed out. I would guess four calls to the db?

Then in the controller:

        var ordersummary = new OrdersSummary();
        var viewModel = new OrderSummary
        {
            Orders = ordersummary.GetOrders(),
            GrossProfitTotal = ordersummary.GetGrossProfitTotal(),
            CommissionTotal = ordersummary.GetCommissionTotal(),
            NetProfitTotal = ordersummary.GetNetProfitTotal(),
        };
        return View(viewModel);

This gets me all the data I need in the view so I can work with it. To me, it just seems unnecessarily redundant and I'm guessing inefficient? If you throw in that I'm also doing sort and search parms, it's a lot of duplicate linq code as well. It seems like I should be able to do something to consolidate the data like this:

   var orders = (from s in db.Orders
                 where s.UserId == uId
                 select s).ToList();

   decimal grossprofittotal = orders.Sum(s => s.Profit);
   decimal commissiontotal = orders.Sum(s => s.Commission);
   decimal netprofittotal = orders.Sum(s => s.Profit + s.Commission);

and then wrap those four pieces of data (orders list, and three decimal values) up nicely in an array (or whatever) and send them to the controller/view. In the view I need to be able to loop through the orders list. Am I way off here? Or, what is standard procedure here with MVC? Thanks.

Answer 1

Yes, fetching the same data four times is indeed inefficient, and completely unneccesary. You can very well fetch it only once and then do the other operations on the data that you have.

You can keep the GetOrders method as it is if you like, but that's all the data that you need to fetch. If you fetch the data in the controller or in the model constructor is mostly a matter of taste. Personally I tend to put more logic in the model than the controller.

As long as you use ToList to make sure that you actually fetch the data (or any other method that realises the result as a collection), you can calculate the sums from what you have in memory. (Without it, you would still be doing four queries to the database.)

Instead of summing up the profit and commision from all items to get the net profit total, you can just calculate it from the other sums:

decimal netprofittotal = grossprofittotal + netprofittotal;

Answer 2

LinqToEntities tranlates all query into SQL. If you don't want to make more than one transaction, you can fetch the result into a variable by .ToList(),querying this object make the calculation by linqToObject in the memory.

Backward: It fetchs all orders from database first.

 var ordersInMemory = orders.ToList();
 decimal grossprofittotal = ordersInMemory.Sum(s => s.Profit);
 decimal commissiontotal = ordersInMemory.Sum(s => s.Commission);
 decimal netprofittotal = grossprofittotal + commissiontotal ;