Is Deferred Execution in ASP.NET MVC View a Very Bad Thing

Is Deferred Execution in Asp.net MVC View a very bad thing?

Deferred execution means, your actual SQL query behind the LINQ expression won't be executed until you start accessing the items in the users collection (when you iterate those in the foreach loop). That means, There will be a SELECT query executed if you are trying to access a navigational property (and it is a unique record) on the User entity

If you only have a single user table (like what you have showed in your question) without any foreign keys/vanigational properties, With the above code you have, EF will execute only one query to get data from your User table. But typically this is not the case, You might have foreign keys to a lot of different tables from the User table. In that case Entity framework does thing differently.

Let's take an example.

Assume that we have a second table, UserType which has 4 columns, Id, Name ,Code and IsAdmin and your User table has a third column called UserTypeId which has a foreign key to this new UserType table. Every user record is associated with a UserType record.

Now you want to display all the Users with the UserType name. Let's see different approaches.

1. Deferred execution,

You will execute this code to get all the users

var users = dbContext.Users;

And pass the users to the razor view where you will iterate through the collection.

@model IEnumerable<YourEntityNameSpace.User>
@foreach (var user in Model)
{
    <div>
        @user.Name - @user.UserType.Name
    </div>
}

When we execute this page, Entity framework is going to run 1 select query on the User table to get the user records along with the UserTypeId and when the foreach block is being executed, it is going to query the UserType table for each unique UserTypeId from the original result set(users). You can see this if you run a SQL profiler.

You can see that EF is passing UserTypeId (2 in my picture). I had 3 different UserTypeId's being used in the User table, so it queried the UserType table 3 times, one for each UserTypeId.

Number of SQL queries executed : 4 (1 for user table + 3 for UserType table)

I had 3 different records in the UserType table and i used all those in my User Table.

2. Using Include method when querying data

Include keyword is used to achieve eager loading. Eager loading is the process where a query for one type of entity also loads related entities as part of the query.

var users = dbContext.Users.Include(s=>s.UserType);

Here you are telling Entity framework to query from UserType table along with User table. Entity framework will produce an INNER JOIN sql query between both the tables and execute it.

You can see that, It queried all the columns of the UserType table)

Number of SQL queries executed : 1

A Better solution ?

Usually, It is not a good idea to use the entity classes generated by Entity framework in other layers so much. That makes your code tightly coupled.
I would suggest querying only data(columns) which is needed and map that to a POCO class (A simple DTO) and use that in your views /other layers. You may keep these DTO classes in common project which can be referred in other projects (Your Data Access project and UI project)

public class UserDto
{
  public int Id {get;set;}
  public string Name {get;set;} 
  public UserTypeDto UserType { set; get; }    
}
public class UserTypeDto
{
    public int Id { set; get; }
    public string Name { set; get; }
}

Our view will be bound to a collection of UserDto insted of the User entity

@model IEnumerable<YourCommonNamespace.User>
@foreach (var user in Model)
{
    <div> @user.Name - @user.UserType.Name </div>
}

And now from your Data access layer, you will be returning a collection of UserDto instead of the User entity created by Entity framework.

var users = dbContext.Users.Select(s => new UserDto
{
    Id = s.Id,
    Name = s.Name,
    UserType = new UserTypeDto
    {
        Id = s.UserType.Id,
        Name = s.UserType.Name
    }
});

Here, You can see that We are using the Select clause to tell EF which columns we really need. EF will execute an INNER JOIN, but with only those columns, we specified

Number of SQL queries executed : 1

The benefit of this approach is, If you ever want to switch your data access implementation from Entity framework to some other technologoy (Pure ADO.NET/ NHibernate) for your own reasons, You will be only updating your GetUsers method, All other layers (your Razor views/ Other Business layer code etc..) don't need an update because they are not using the entities created by entity framework.

If you do a ToList(), EF executes the SQL right away and return the result. But instead if you are not doing that, because of deferred execution, It is going to execute the SQL statement(s) when it actually needs the data (ex : You render some property value in your view inside the loop).

Does LINQ deferred execution occur when rendering the view, or earlier?

MSDN documentation addresses this question under the deferred query execution section (emphasis mine).

In a query that returns a sequence of values, the query variable
itself never holds the query results and only stores the query
commands. Execution of the query is deferred until the query variable
is iterated over in a foreach or For Each loop...

That narrows down the answer to options 2 and 3.

foreach is just syntactic sugar, underneath the compiler re-writes that as a while loop. There's a pretty thorough explanation of what happens here. Basically your loop will end up looking something like this

{
  IEnumerator<?> e = ((IEnumerable<?>)Model).GetEnumerator();
  try
  { 
    int m;  // this is inside the loop in C# 5
    while(e.MoveNext())
    {
      m = (?)e.Current;
      // your code goes here
    }
  }
  finally
  { 
    if (e != null) ((IDisposable)e).Dispose();
  }
}

Enumerator is advanced before it reaches your code inside the loop, so slightly before you get to @item.Bar. That only leaves option 2, the @foreach (var item in Model) line (though technically that line doesn't exist after the compiler is done with your code).

I'm not sue if the query will execute on the call to GetEnumerator() or on the first call to e.MoveNext().

---

As @pst points out in the comments, there are other ways to trigger execution of a query, such as by calling ToList, and it may not internally use a foreach loop. MSDN documentation sort of addresses this here:

The IQueryable interface inherits the IEnumerable interface so that if
it represents a query, the results of that query can be enumerated.
Enumeration causes the expression tree associated with an IQueryable
object to be executed. The definition of "executing an expression
tree" is specific to a query provider. For example, it may involve
translating the expression tree to an appropriate query language for
the underlying data source. Queries that do not return enumerable
results are executed when the Execute method is called.

My understanding of that is an attempt to enumerate the expression will cause it to execute (be it through a foreach or some other way). How exactly that happens will depend on the implementation of the provider.

Should I use ViewBag to pass a list to a View?

Start by creating the view models to represent what you want to display/edit in the view. Your PatrolMemberViewModel can be used but remove the [Key] attribute and the int PatrolId and PatrolViewModel Patrols properties.

Then create the parent view model

public class AssignPatrolViewModel
{
    [Display(Name = "Patrol")]
    [Required(ErrorMessage = "Please select a patrol")]
    public int? SelectedPatrol { get; set; }
    public IEnumerable<SelectListItem> PatrolList { get; set; }
    public List<PatrolMemberViewModel> Members { get; set; }
}

and you GET method would be

public ViewResult Unassigned()
{
  var model = new AssignPatrolViewModel
  {
      PatrolList = new SelectList(db.Patrols, "PatrolId", "PatrolName"), // modify to suit
      Members = repository.SelectAllUnassigned().ToList()
  };
  return View(model);
}

and in the view

@model AssignPatrolViewModel
....
@using (Html.BeginForm())
{
    @Html.LabelFor(m => m.SelectedPatrol)
    @Html.DropDownListFor(m => m.SelectedPatrol, Model.PatrolList, "Please select")
    @Html.ValidationMessageFor(m => m.SelectedPatrol)
    <table>
        .... // thead elements
        <tbody>
            @for(int i = 0; i < Model.Members.Count; i++)
            {
                <tr>
                    <td>
                        @Html.CheckBoxFor(m => m.Members[i].IsSelected)
                        @Html.HiddenFor(m => m.Members[i].MemberId)
                        // add other hidden inputs for properties you want to post
                    </td>
                    <td>@Html.DisplayFor(m => m.Members[i].FirstName)</td>
                    ....
                </tr>
            }
        </tbody>
    </table>
    <input type="submit" value="Assign" class="btn btn-success" />
}

Then in the POST method

[HttpPost]
public ViewResult Unassigned(AssignPatrolViewModel model)
{
    if (!ModelState.IsValid)
    {
        model.PatrolList = new SelectList(db.Patrols, "PatrolId", "PatrolName");
        return View(model);
    }
    // Get selected ID's
    IEnumerable<int> selected = model.Members.Where(m => m.IsSelected).Select(m => m.MemberId);
    // Get matching data models
    var members = db.Person.Where(p => selected.Contains(p.PID)); // modify to suit
    // loop each each member, update its PatrolId to the value of model.SelectedPatrol
    // save and redirect
}

Entity Framework - Using Statement w/ Deferred Execution in a Repository

It's fine to return IQueryable<T>, but wrong to either not dispose an ObjectContext (you should always dispose any IDisposable) or to dispose it before the request is complete.

Use constructor injection to pass the context to the repository and dispose it at the end of the request.

Doesn't nhibernates suggestion of putting everything in a transaction work against the idea of lazy loading?

You should create ViewModels that represent everything that the View needs to be rendered. You should not be issuing database queries from the view if at all possible. To summarize the link above here:

• It increase the time that the connection to the database have to be
  open. The recommendation is to keep that open only for the duration of
  the action, not throughout the lifetime of the request.
• It make it
  that much harder to understand what are the data requirements for a
  particular action is.
• When writing views, you shouldn't be bothered
  with thinking about persistence, or the number of queries that you
  views are generating.
• The views are often the most changeable parts in
  the application, and having the application issue queries from the
  views may result in significant changes to the way the application
  data access pattern between revisions.
• Most often, queries from the
  views result from lazy loading, Select N+1 or similar bad practices.

We strongly recommend that you'll avoid generating queries in the
view, instead, perform all your queries in the action, and provide in
memory access only to the view for them to render themselves.

(Emphasis on the last point mine).

The practical implications of this are that you should not be lazy loading anything in a view. So what should you do instead? This is where the ViewModel layer comes in. You should be fully populating a thin ViewModel with the information you need and then rendering the view with that.

Furthermore, ViewModels shouldn't even contain classes mapped with NHibernate (this appears to be what you're doing in your example).

With all this in mind I would change your current code:

public class CacheTestViewModel 
{
    public List<ProjectViewModel> Projects { get; set; }
}

public class ProjectViewModel
{
    public string Owner { get; set; }
    /* etc. */
}

.. And then your code to populate those ViewModels:

public CacheTestViewModel GetCacheTestViewModel()
{
    var vm = new CacheTestViewModel();
    var session = Repository.Session;
    using (var tx = session.BeginTransaction())
    {
        var projects = Repository.Session.Query<Project>().Cacheable();

        foreach (var project in project)
        {
            vm.Projects.Add(new ProjectViewModel { Owner = project.Owner.Name });
        }

        tx.Commit();
    }
    return vm;
}

Now you might be thinking "gee, that's a lot of code to map domain entities to ViewModels. I can see that getting out of hand." You'd be right about that. There's a library called AutoMapper that you can use to semi-automate your mappings.

Where should my filtering logic reside with Linq-2-SQL and ASP.NET-MVC in View or Controller?

It's a subject of great debate I think. To me there are two clear routes:

Deferred Execution Route
Never use ToList, always use AsQueryable in controllers. This means only the data that is necessary for presentation is retrieved and only when it is needed in the View once ToList etc. are called.

Presentation Shows Models
Prepare data for presentation using Model classes or results that are already filtered. Strongly type your View to the expected format.

I prefer deferred execution myself, though in a pure MVC structure the Controller should worry about getting the information and the View should only display the Mode.

Regardless, make sure you are using AsQueryable with your Repository.

---

Related Topics