Use of using statement for Entity Framework DbContext in foreach-loop

Question

There is a parser that parses a text file which contains object definition. The object definitions in the text file have a placeholder handle key. The place holder handle needs to be replaced with actual value by looking up the handle value in DB. In my application I am making use of the Entity framework Core for working with the DB.

The parser returns one object at a time, and I am looking up the handle and other properties in the DB one at a time. This is how the code looks so far:

    IEnumerable<ObjectInfo> GetNextContent();

    IEnumerable<ObjectInfo> GetNextObjectInfo()
    {

        foreach (var item in parser.GetNextContent())
        {
            using (var dbContext = new ContentDbContext())
            {
                string key = item.Key;

                string id = dbContext.Contents.Find(key).ObjectId;
                item.Id = id;
                // Assign other fields...

                yield return item;
            }
        }
    }

The question that I have is that in the code above, the 'using' block is within the foreach loop. Is this a right thing to do? The other thought is that I can take the 'using' block outside of the foreach-loop but then I am not sure how would that play out with the iterator in the code.

You can do it, the question is whether you should. You are always better to do things in batches when you can — TheGeneral, Feb 18 '20 at 04:12
Right, this is precisely my question. I am concerned am I causing too many dispose() calls when using the DbContext within the loop. — tyrion, Feb 18 '20 at 04:17
Putting the context outside for an extended period of time can create its own set of issues. My point was mainly, that you are better off to get a list of Ids, and query the database in one go or in batches (if possible), or if the list isn't too large, load them all into memory. it all really depends on information you haven't supplied — TheGeneral, Feb 18 '20 at 04:20
@Michael Randall The same my idea. Check [my answer](https://stackoverflow.com/a/60273644/9071943) to be more details. — Nguyễn Văn Phong, Feb 18 '20 at 04:25
@MichaelRandall Yes, that makes sense. I can perform this in batch as well, thanks for the pointer. As for number of times, the loop might run, the count can vary from 10K to 20K.Doing this now within the for-each loop looks bad idea. — tyrion, Feb 18 '20 at 04:33

score 3 · Accepted Answer · edited Feb 18 '20 at 05:46

3

You should move ContentDbContext into outside for better performance.

This is simply because You just need one context per request.

One DbContext per web request... why?

using (var dbContext = new ContentDbContext())
 {
    foreach (var item in parser.GetNextContent())
     {
            string key = item.Key;

            string id = dbContext.Contents.Find(key).ObjectId;
            item.Id = id;
            // Assign other fields...

            yield return item;
        }
    }

Updated

You might also join then make sure that fetch all data at a time

// You need to fetch all `item.Key` from `parser.GetNextContent()` to get all data in `dbContext.Contents`
var keys = parser.GetNextContent().Select(p => p.Key).ToArray();

var result = (from content in dbContext.Contents 
              join key in keys on content.Id equals key 
              select new 
{
  Id = content.ObjectId,
  //....
}

If you are use C# 8, using statement may be as below:

 using var dbContext = new ContentDbContext();

 foreach (var item in parser.GetNextContent())
 {
        string key = item.Key;

        string id = dbContext.Contents.Find(key).ObjectId;
        item.Id = id;
        // Assign other fields...

        yield return item;
  }

edited Feb 18 '20 at 05:46

Ramil Aliyev 007

4,437
2
31
47

answered Feb 18 '20 at 04:12

Nguyễn Văn Phong

13,506
17
39
56

This LINQ query looks very interesting. Based on comments from @MichaelRandall I realized I would need to batch the items coming from parser and do a single lookup in DB. Does the above query doesn't need that anymore? – tyrion Feb 18 '20 at 04:41
Note in the last example `parser.GetNextContent()` will need to be enumerated first. `ToList()` as it will likely throw an error because it cant be converted to *sql* – TheGeneral Feb 18 '20 at 04:44
_because You just need one context per request._ - context is just a c# class as others, instantiating it multiple times will not cause more problem than instantiating some other disposable instance. – Fabio Feb 18 '20 at 04:46
1

@Michael Randall I've just updated my answer, please take a look at – Nguyễn Văn Phong Feb 18 '20 at 04:49
@Fabio Yes, `disposable` would be a problem as well. In my opinion, the second solution is much better – Nguyễn Văn Phong Feb 18 '20 at 04:51
1

@Phong Thanks for the updated answer. I will make use of the 2nd solution. – tyrion Feb 18 '20 at 04:53
Yes, It's my pleasure. – Nguyễn Văn Phong Feb 18 '20 at 04:54
@Phong, linked answer is a bit outdated, and suggestion to use one context per request is based on assumption of particular design of your application, but technically there are **no restrictions** on using transient context. Transient context provide more options for it consumption, for example asynchronous queries, where wich single context will not be able to execute two asynchronous queries "simultaneously". – Fabio Feb 18 '20 at 06:21

Use of using statement for Entity Framework DbContext in foreach-loop

1 Answers1