Questions about Entity Framework Context Lifetime

I have some questions about the desired lifetime of an Entity Framework context in an ASP.NET MVC application. Isn't it best to keep the context alive for the shortest time possible?

Consider the following controller action:

public ActionResult Index()
{
    IEnumerable<MyTable> model;

    using (var context = new MyEntities())
    {
        model = context.MyTable;
    }

    return View(model);
}

The code above won't work because the Entity Framework context has gone out of scope while the view renders the page. How would others structure the code above?

Solution 1:

Let's get controversial!

I disagree with the general MVC + EF consensus that keeping a context alive throughout the entire request is a good thing for a number of reasons:

Low performance increase Do you know how expensive creating a new database context is? Well... "A DataContext is lightweight and is not expensive to create" that's from MSDN

Get the IoC wrong and it'll seem fine.. until you go live If you set up your IoC container to dispose of your context for you and you get it wrong, you really really get it wrong. I've twice now seen massive memory leaks created from an IoC container not always disposing of a context correctly. You won't realise you've set it up wrong until your servers start crumbling during normal levels of concurrent users. It won't happen in development so do some load tests!

Accidental lazy loading You return an IQueryable of your most recent articles so that you can list them on your homepage. One day someone else is asked to show the number of comments next to the respective article. So they add a simple bit of code to the View to show the comment count like so...

@foreach(var article in Model.Articles) {
    <div>
        <b>@article.Title</b> <span>@article.Comments.Count() comments</span>
    </div>
}

Looks fine, works fine. But actually you didn't include the comments in your returned data so now this will make a new database call for each article in the loop. SELECT N+1 issue. 10 article = 11 database calls. Okay so the code is wrong but it is an easy mistake to make so it will happen.

You can prevent this by shutting your context down in you data layer. But won't the code break with a NullReferenceException on the article.Comments.Count() ? Yes it will so it will force you to edit the Data layer to get the data needed for the View layer. This is how is should be.

Code smell There is just something wrong about hitting the database from your View. You know that an IQueryable hasn't actually hit the database yet right so forget that object. Make sure your database is hit before it leaves your data layer.

So the answer

Your code should be (in my opinion) like this

DataLayer:

public List<Article> GetArticles()
{
    List<Article> model;

    using (var context = new MyEntities())
    {
        //for an example I've assumed your "MyTable" is a table of news articles
        model = (from mt in context.Articles
                select mt).ToList();
        //data in a List<T> so the database has been hit now and data is final
    }

    return model;
}

Controller:

public ActionResult Index()
{
    var model = new HomeViewModel(); //class with the bits needed for you view
    model.Articles = _dataservice.GetArticles(); //irrelevant how _dataService was intialised
    return View(model);
}

Once you have done this and understand this then perhaps you can begin experimenting with having an IoC container handle context but definitely not before. Head my warning - I've seen two large scale failures :)

But honestly do what you like, programming is fun and should be a matter of preference. I'm just telling you mine. But whatever you do, don't start using IoC context per controller or per request just because "all the cool kids are doing it." Do it because you really truly care about it's benefits and understand how it's done correctly.

Solution 2:

I agree with one context per request, we normally do this by binding the context .InRequestScope using Ninject, which works really well, is:

Bind<MyContext>().ToSelf().InRequestScope();

Also its really good practice to enumerate the set as close to the query as possible ie:

public ActionResult Index()
{
    IEnumerable<MyTable> model;

    using (var context = new MyEntities())
    {
        model = (from mt in context.MyTable
                select mt).ToArray();
    }
    return View(model);
}

this will help you avoid augmenting the query unintentionally from your view.

Solution 3:

Firstly, you should consider putting your database access to separate classes.

Secondly, my favorite solution to this is to use "one context per request" (if you're using MVC, I believe it's one context per controller).

Requested edit:

Have a look at this answer, maybe it will help you too. Please note I'm using webforms, so can't verify it in MVC at the moment, but it may be helpful for you or at least give you some pointers. https://stackoverflow.com/a/10153406/1289283

Some example usage of this dbcontext:

public class SomeDataAccessClass
{
    public static IQueryable<Product> GetAllProducts()
    {
        var products = from o in ContextPerRequest.Current.Products
                       select o;
        return products;
    }
}

Then you can do something like this:

public ActionResult Index()
{
     var products = SomeDataAccessClass.GetProducts();
     return View(products);
}

Simple, right? You don't have to worry about disposing your context anymore, you write only the code you really need.

Some people like to further spice things up a little bit by adding UnitOfWork pattern, or maybe IoC containers... But I like this approach more because of its simplicity.

Solution 4:

Can you utilize LINQ's .ToList() extension method as such:

public ActionResult Index()
{
    IEnumerable<MyTable> model;

    using (var context = new MyEntities())
    {
        model = (from mt in context.MyTable
                select mt).ToList();
    }
    return View(model);
}