-1

I'm being told that putting a direct reference to IQueryable from an ASP.NET MVC view is a bad practice but I have not found any clear explanation for that. Some might have experienced a "disposed object" error when accessing an IQueryable or a DbContext in a view but it is not a problem I am concerning about.

Here is how I implement a simple controller and a view that contains a list of users

public ActionResult Index() 
{
    return View(db.Users);
}

In my view

@model IEnumerable<User>

<ul>
@foreach (var user in Model)
{
    <li>@user.UserName</li>    
}
</ul>

And I'm being told that I should return a "collection object" instead of passing IQueryable to the view.

public ActionResult Index() 
{
    return View(db.Users.ToList());
}

I'm a little bit curious about why I have to do that. I can say that the latter approach is worse than my first approach because the dataset is iterated twice in my application, first to construct the List object by the ToList() method, second by the foreach loop in my view to render <li> items.

So that means the latter approach must have a better point somewhere that I have not found out yet. Even in ASP.NET forum or Microsoft MSDN site, they also provides examples of the latter approach but did not give any reason on that, or am I missing something?

Can any one explain this simple basic stuff for me?

Many thanks.

tereško
  • 58,060
  • 25
  • 98
  • 150
Nguyen Thanh Ha
  • 238
  • 1
  • 5
  • 1
    You shouldn't use an IQueryable because you havent materialize the query yet so every time you materialize you will be making a call to the database. If you dispose of you db first and you try to materialize you wont be able to access the underlying db – johnny 5 Mar 10 '16 at 15:21
  • @BradleyUffner no hes reffering to using IQueryable in his views instead of a view model – johnny 5 Mar 10 '16 at 15:22
  • Because controllers are created for every request, so if I added the logic to dispose of the DbContext to the destructor or Dispose() method of a controller if following IDisposable pattern then the DbContext will be disposed whenever the Controller is disposed. And what is the meaning of keeping an IQueryable alive while your controller is being disposed and your view is being forgotten (as your application is shutting down for example) ? http://stackoverflow.com/questions/5425920/asp-net-mvc-controller-created-for-every-request – Nguyen Thanh Ha Mar 10 '16 at 16:11

1 Answers1

4

If you pass IQueryable you are creating a leaky abstraction where you can essentially move business logic in the view instead of strictly rendering the business model. This is because IQueryable has not yet been executed against the database which leaves a lot of options to the view like filtering more or retrieval of additional properties etc which should really be done in the Controller (good Separation of Concerns).

To your 2nd point. If you are returning so many objects that iterating over them in a loop (for/foreach/etc) causes such performance degradation than you have much bigger problems like how to even go about sending/rendering that huge list in HTML you are building which in combination with the actual retrieval of such a list from the DB would be the bottle necks.

Finally you need to dispose of the underlying DbContext. This could be registered with your controller for cleanup when your controller is disposed but I could see many programmers forgetting to do it thus leaving open database connections.

Igor
  • 60,821
  • 10
  • 100
  • 175
  • I got your idea about Separation of Concerns and leaky abstraction. Thanks for your explanation. How about if I create a wrapper Enumerator and an extension method to cast an IQueryable to my wrapper Enumerator so that the instance passed to the view is actually an IEnumerable but the query is still not executed until a foreach or a GetEnumerator() is called? – Nguyen Thanh Ha Mar 10 '16 at 16:18
  • @NguyenThanhHa - Why would you want that? Iterating over the collection in memory is fairly fast. If your collection is very large then the slowest part will be the retrieval from the Database and not looping over it. If the collection is very large and Db retrieval is very slow because of it you should add some type of paging or filtering logic to the controller. I see no benefit to having an open `IQueyrable` object (wrapped or not) to a view especially none that have to do with performance. – Igor Mar 10 '16 at 16:23
  • A view can be a `JsonResult` which contains list of data. It is not always an HTML view. What I concern is, using `ToList()` means you put all data in the application memory which looks like a "buffering" fashion; and that means memory intensive. While using `IQueryable`, you are iterating through each item, one by one loaded to application memory, which looks like a "streaming" fashion. Is it correct? – Nguyen Thanh Ha Mar 10 '16 at 17:01
  • @NguyenThanhHa - 1st point. `A view can be a JsonResult which contains list of data` - no, it can't. A view as a ViewResult and not JsonResult. An `ActionResult` from a controller could be a `JsonResult` or a `ViewResult` and even though both have a common base class `ActionResult` that is where the relationship stops. JsonResult and ViewResult are not the same. Also a JsonResult is not built up in a razor view (*unless you are using ViewResult in a mangled way*), these are built up and returned in the controller. – Igor Mar 10 '16 at 18:50
  • @NguyenThanhHa - your 2nd point. Yes, iterating over the collection without using AsEnumerable/ToList/ToArray will ensure that the objects are not persisted to memory first. Again, most likely this is a micro improvement unless you are retrieving an enormous amount of data. If that is the case then you should be adding some type of paging/filtering logic because it will also be slow retrieval from the server (db hit either way) AND sending it back to the client. Finally you could call your IEnumerable multiple times = multiple DB calls which is not the case with a List. – Igor Mar 10 '16 at 18:53
  • 1
    Ok got it now. Many thanks for your attention and your clear explanations. :) – Nguyen Thanh Ha Mar 10 '16 at 18:59
  • 1
    I have just thought of a case that can prove passing an `IQueryable` to a view is not a good practice. Considering the case when there are different developers working on controllers and views. So when passing an `IQueryable` to a view, the developers working on controller would not know how the developers will use that instance in their view implementation. So for a safe, and good practice, they should provide a collection object to avoid the case one database query can run multiple times regardless of how the view developers do. – Nguyen Thanh Ha Mar 10 '16 at 19:32