Correct way of using the 'using' statement in Entity Framework

Question

I have some questions about using the using statement. I understand what it does (If I'm correct, it disposes all open connections and such), but I'm unsure how I should use it correctly.

The project I'm working in doesn't contain any repositories, which you don't need for Entity Framework.

So basically, I'm getting a list of Guids as parameter in my method. These ids are for restaurants, and I want to retrieve all reviews that have been given on these restaurants.

So currently, I'm retrieving the list like so:

public void DoSomething(List<Guid> restaurantIds)
{
    List<Review> reviews;
    using (var db = new Context())
    {
        reviews = db.Reviews.Where(x => restaurantIds.Contains(x.RestaurantId)).ToList();
    }
    //More stuff here
}

Is this a common bad practice to declare the list outside of the using statement? I thought of a few alternatives, but I'm (again) unsure what would be better.

1. Create a seperate method in the same class which does exactly that and returns the list. This way in my DoSomething method, I can just use it like this: List<Review> reviews = GetReviewsFromRestaurants(restaurantIds);
2. I have to create the context first, and then use the LINQ statement without the using block. I have to call .Dispose() when I'm done with it though.

Is there a problem when I use the using statement like in my example? If so, are the alternatives better? And if that's not the case, could you give me an example on how I should retrieve this list?

Answer 1

Here in this case braces defines their own scope. The variables which you will declare outside the scope of braces will be visible inside braces and its okay.

Its actually the shorthand for try catch block.

List<Review> reviews;
var db = new Context();
try
{
   reviews = db.Reviews.Where(x => restaurantIds.Contains(x.RestaurantId)).ToList();
}
finally
{
  db.Dispose();
} 

And your snippet is much more concise than this. Compiler will always call .Dispose on the "used" object.

Answer 2

About the reviews variable, I do not think it is bad that you declare it outside of using block.

1. Separate method is recommended. Actually you can still use a repository class (RestaurantRepository) that will handle all these basic operations like: get all restaurants based on a single or multiple identifier, create a new restaurant, change some data for a restaurant.

This ensures that you separate your business logic from basic operations.

2. Disposable context vs. explicit Dispose(). Disposable context is clearly better since it you make sure that Dispose() is called even if your code fails.

Bigger picture - Entity framework and context dispose

This is already discussed here. Also, this article shows that dispose is not so necessary as it seems.

Personally, I have been Unit of Work pattern to allow multiple changes (on various repositories / entities) and did not get into trouble for not disposing the context.

Answer 3

There usually is no one-size-fits-all answer to "what is a bad practice" and your question is no exception. While often thare are clearly bad prectices and sometimes clearly good practices, there is a lot of factors that influence your decision (such as your requirements, the rules of your organization or the experience of your team).

You can keep a context in entity framework for longer than just a moment (although keeping it open for very long might also not be a good idea), as long as you are sure you dispose it at the end. But there is also nothing wrong in that way you apply using to your context. In web applications contexts are often used for a very short time and low-level connection pooling usally keeps the solution still performant.