Foreach loop async-await

Question

I have a question about good practice with async-await in foreach loop.

My method looks like this

 public async Task<List<Category>> GetCategoriesAsync(string partOfName,
        CancellationToken ct = default(CancellationToken))
    {
        string lowerCasePartOfName = partOfName.ToLower();

        var categories = await _context.ECATEGORIES
            .Where(a => a.NAME.ToLower().Contains(lowerCasePartOfName))
            .ProjectTo<Category>()
            .ToListAsync(ct);

        //version1 #Beginning 
        var i = 0;
        foreach (var parentId in categories)
        {
            var categoryParent = await _context.ECATEGORIES
                .Where(a => a.ID == parentId.ParentId)
                .Select(s => s.NAME)
                .FirstOrDefaultAsync(ct);

            categories[i].CategoryParent = categoryParent;
            i++;
        }
        //version1 #End

        //version2 #Beginning 
        categories.ForEach(async x => x.CategoryParent = await _context.ECATEGORIES
            .Where(a => a.ID == x.ParentId)
            .Select(s => s.NAME).FirstOrDefaultAsync(ct));
        //version2 #End

        return categories;
    }

Version1 and version2 gives same result but I would like to ask which is better practice for async tasks or maybe none of them.

Thanks in advance.

Both are bad in that the code could be rewritten to make use of a proper join. That would make 1 db call instead of 1 call per category + 1 (for the initial call). But strictly answering your question, it does not matter: pick the one you feel most comfortable with. — Igor, Nov 21 '18 at 10:13
Problem I had is that, parentId is often duplicated and I couldn't use .Contains — KJanek, Nov 21 '18 at 10:16

Igor · Answer 1 · 2018-11-21T11:31:47.060

1

Both are bad in that the code could be rewritten to make use of a proper join. That would make 1 db call instead of 1 call per category + 1 (for the initial call). But strictly answering your question, it does not matter: pick the one you feel most comfortable with

Problem I had is that, parentId is often duplicated and I couldn't use .Contains

You can use a left join to do the same thing but all in 1 DB call which is cheaper than 1 call per result.

string lowerCasePartOfName = partOfName.ToLower();

var categories = await (from category in _context.ECATEGORIES.Where(a => a.NAME.ToLower().Contains(lowerCasePartOfName))
    from parent in _context.ECATEGORIES.Where(parent => parent.ID == category.ParentId).DefaultIfEmpty()
    select new Category
    {
        Id = category.ID,
        CategoryParent = parent.NAME,
    }).ToListAsync();

If your schema is setup to be case insensitive then you can omit the ToLower calls as well. You can check this by looking at the COLLATION.

.Where(a => a.NAME.Contains(lowerCasePartOfName))

edited Nov 21 '18 at 11:31

answered Nov 21 '18 at 10:24

Igor

60,821
10
100
175

Sorry but I don't know how to use it in my method:/ In your code parent is IEnumarable so I can't reference like parent.NAME . – KJanek Nov 21 '18 at 11:15
in braces {} property parent still stays as a collection. – KJanek Nov 21 '18 at 11:25
@janek9971 - give me 1 second, I will fix it. – Igor Nov 21 '18 at 11:26
var categories = await _context.ECATEGORIES .Where(a => a.NAME.ToLower().Contains(lowerCasePartOfName)) .GroupJoin(_context.ECATEGORIES, child => child.PARENTID, parent => parent.ID, (category, parent) => new { Category = category, ParentName = parent.FirstOrDefault()}) .Select(s => new Category { Id = s.Category.ID, CategoryParent = s.ParentName.NAME, ParentId = s.Category.PARENTID, }) .ToListAsync(ct); – KJanek Nov 21 '18 at 11:31
I think now is ok? If yes I post as my answer to my question. I changed parent.Name to parent.FirstOrDefault() and then retrieve Name – KJanek Nov 21 '18 at 11:32
@janek9971 - that works but I rewrote it using linq instead of lambda which is easier to read (IMO). Either one will work. – Igor Nov 21 '18 at 11:32
@janek9971 - if you are curious how that works see https://stackoverflow.com/a/23558389/1260204 – Igor Nov 21 '18 at 11:33
@janek9971 - glad to help. Please consider marking an answer if this solves your problem (answers your question). – Igor Nov 21 '18 at 11:39
I have a last question is it possible to still use automapper ProjectTo<> in this case? – KJanek Nov 21 '18 at 11:42
@janek9971 - I am not sure, I am not familiar with AutoMapper – Igor Nov 21 '18 at 11:51
Upon reflection, I think that is not possible without adding ForMember in CreateMap cause CategoryParent is not get in a straight way which would be more unreadable. – KJanek Nov 21 '18 at 11:58

Foreach loop async-await

1 Answers1