Asp Net MVC View performance and code logic

Question

I have a view which displays products and the corresponding’s category details in my application. It’s inheriting from a viewmodel in order to fetch both products and category info that I need to display.

public ViewResult Index(string id, int page = 1, int pagesize = 10)
    {
        var viewModel = new PagedProductList()
        {
            CurrentPageIndex = page,
            PageSize = pagesize
        };

        viewModel.category = db.Category.First(c => c.Id == id);

        viewModel.products = db.Product
            .Where(i => i.Category.Id == viewModel.category.Id)
            .OrderBy(i => i.Id)
            .Skip((viewModel.CurrentPageIndex - 1) * viewModel.PageSize)
            .Take(viewModel.PageSize)
            .Include(p => p.ProductAttributeValues);

        return View(viewModel);
    }

As you can see I’m passing the viewmodel's category id to the viewmodel’s products where clause.

Question 1: Would this work better in terms of performance or some other way if I moved the products code in a separate child action and then call the action from the parent view using Html.Action passing the category model id from the parent.

Question 2: Is there any other better way to fetch both the category info and the corresponding products. I can’t use something like db.Category.Include(“Products”) because I won’t be able to do paging on the products.

I think you misunderstand the purpose of a view model. You should not be putting your database access inside it. Having said that, are you having any particular performance problems? If not, then further optimization is unnecessary. — Erik Funkenbusch
– Erik Funkenbusch, Commented Jan 23, 2013 at 20:00
Well actually i'm not putting database access in the viewmodel. This is done in the controller's code i posted. And is temporary because i will move it in a repository after i'm done assessing it. — Panos
– Panos, Commented Jan 23, 2013 at 20:58
Actually, Yes, you are. WHen you assign the query to Products, you're assigning an IQueryable, which actually executes database calls as you iterate over it, which will be in your view. It is also returning Entities to your view, which completely misses the point of the view model. — Erik Funkenbusch
– Erik Funkenbusch, Commented Jan 23, 2013 at 21:41
Ok i get it i have to assign the query to a var and the result to the viewmodel right? — Panos
– Panos, Commented Jan 23, 2013 at 21:54
No. A var doesn't change anything, it's just an implicit type definition. But that's all beside my point. Are you actually having performance problems? If not, then why are you worrying about it? — Erik Funkenbusch
– Erik Funkenbusch, Commented Jan 23, 2013 at 22:24

Bennor McCarthy · Accepted Answer · 2013-01-23 20:15:40Z

Question 1: No. It would be slower. There is an overhead associated with calling actions (i.e. finding and setting up controllers). The end result would still be issuing two queries.

Question 2: You can get what you want (navigation through a category's products with sorting and paging) in a single query using projection:

var data = db.Category
             .Select(c => new 
             {  
                c, 
                Products = c.Products
                            .OrderBy(p => p.Id)
                            .Skip((viewModel.CurrentPageIndex - 1) * viewModel.PageSize)
                            .Take(viewModel.PageSize)
                            .Select(p => new { p, p.ProductAttributeValues })
            })
            .Single(c => c.Id == viewModel.category.Id);
viewModel.category = data.c;
viewModel.products = data.c.Products.Select(p => p.p).ToList();

This will probably perform slightly better than making two separate queries, but it is unlikely you would notice a difference.

However, you have to weigh the benefits of slightly better performance against the decreased readability of this code. If you don't have an actual performance problem (which I'm guessing you don't), then you are wasting your time optimizing the performance here. You should always wait until you have an actual measurable performance problem before optimizing, and unless there's a significant performance benefit to using more complicated code you should always favour readability.

Note: Using a similar pattern for more complicated queries may actually be slower than issuing two or more queries to get just the data you need.

Collectives™ on Stack Overflow

Asp Net MVC View performance and code logic

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related