2

I apologise if my question seems dumb, I have tried Googling, and not found what I am looking for, so any advice would be appreciated.

I am fairly new to the Idea of MVC, I have been doing Web Forms development for a number of years, but I wanted to give something else a try.

I am using Entity Framework (6) for a database connection, and MVC4 for a web front end.

My question is, how should the DB context instance be handles.

My Controller Action code that is working is this

public ViewResult List(int buildingId)
{
    var model = new Data.Model();
    var query = from r in model.Rooms
                where r.Building.Id == buildingId
                select r;
    /*
    var q2 = model.Buildings.Where(b => b.Id == buildingId).SelectMany(b => b.Rooms);
    var q3 = model.Buildings.Where(b => b.Id == buildingId).First().Rooms;
    */

    return View(query);
}

The commented lines are just other ways I could get the query results I am looking for. Data.Model is the EF Db context.

What I don't like about this is that the context is disposable, and I am not disposing it. In my mind this is lazy and bad practice.

I have tested with model begin disposed, the first change that I require is to return a list of the query, which I don't mind, but then because the context is disposed, on the view I cannot access properties like @item.Building.Description. So if disposing I would need to return exactly what I was to display on the view (there are multiple ways I could do that, so I am not very worried about how).

Another option would be to have a static/shared context somewhere in the project, so all database requests use the same context instance. This is nice from the side that it would then only be using one DB connection, but EF might already be handling that for me, so I wouldn't want to go against using the way EF was designed, if this is how it should be.

So, my question is, what would be considered best practices?

  • Keep working like I am, instantiating a new context, and not disposing.
  • Dispose the context each time, and make sure I return anything I need visible on the view.
  • Use a class that will return a static context instance if it is already instantiated.

Thank you

Yakimych
  • 17,612
  • 7
  • 52
  • 69
JonathanPeel
  • 743
  • 1
  • 7
  • 19

1 Answers1

5

Generally I would recommend to use a more architectured approach and use a separate (data-providing) layer for fetching data from the database. In that case an IoC (inversion on control) container (such as Ninject, Unity, etc.) could handle object lifetime for you, but if you haven't used Dependency Injection patterns before, you have a lot to investigate first.

Having said that, the simple answer is that you should definitely dispose your context objects as soon as possible (in general they should be as shortlived as possible). And the common pattern is

using (var model = new Data.Model())
{
    var buildings = model.Rooms.Where(r => r.Building.Id == buildingId).ToList();
}

You fetch the data right away by calling ToList() and dispose of the context. This avoids the problems you've encountered when returning a query and passing it to the view. As you've noticed, it gets really tricky to control when exactly your DB-query gets executed and when to dispose of the context.

Another option would be to have a static/shared context somewhere in the project, so all database requests use the same context instance - this is a horrible idea and a well-known bad practice/antipattern.

So to sum up, the best practice is working as you describe in Option 2 in your list: Dispose the context each time, and make sure I return anything I need visible on the view.

And as a sidenote, it is also a good practice to use a ViewModel per View (see ASP.NET MVC - How exactly to use View Models)

Community
  • 1
  • 1
Yakimych
  • 17,612
  • 7
  • 52
  • 69
  • Hi Yakimych. In this example I am actually looking for the list of rooms in a building, but that doesn't change the answer given. – JonathanPeel Feb 03 '14 at 11:13
  • @JonathanPeel - yes, sorry, you're right of course. I've removed the `FirstOrDefault` comment from the answer. – Yakimych Feb 03 '14 at 12:46