Simplify linq expression that orders children

Question

I am trying to write a LINQ query that fetches a list of Course entities and their mapped Skill children. The are related with a join table with corresponding CourseId and SkillId. I wish to sort the Skill children with the Weight property and then with the SkillId property. I am using dot net core 2.0.

After looking at similar questions on how to sort/order a list of children to an entity here:

I have come up with this:

// Create query to get all courses with their skills
var coursesWithUnorderedSkills= db.Courses
    .Include(i => i.Skills)
    .ThenInclude(i => i.Skill);

// Order the skills for each course
await coursesWithUnorderedSkills.ForEachAsync(x => x.Skills = x.Skills
                                   .OrderBy(o => o.Weight)
                                   .ThenBy(o => o.SkillId)
                                   .ToList());

// Get a list of courses from the query
var coursesWithOrderedSkills = await q.ToListAsync();

How can this be simplified into a single query and will this query have any unexpected performance issues since I am calling ToList in the ForEachAsync call?

Models

public class Course
{
    [Key]
    public int Id { get; set; }
    public List<CourseSkill> Skills { get; set; }
}

public class CourseSkill
{
    public Course Course { get; set; }
    public int CourseId { get; set; }

    public Skill Skill { get; set; }
    public int SkillId { get; set; }

    public int Weight { get; set; } = 0;
}

public class Skill
{
    [Key]
    public int Id { get; set; }
}

what's the purpose of the second statement? That ToList fills a list that then nobody cares about... — Jorge Y., Feb 17 '18 at 11:02
It is used to make sure the list is ordered by the weight and id, I hope :P — span, Feb 17 '18 at 11:08
I don't think that the collection stays sorted. But anyway, I don't get why don't you just assign q2 as the result of the first await, which is already executed asynchronously. — Jorge Y., Feb 17 '18 at 11:15
I am new to c# and LINQ and I might very well make things more complicated then they really are due to my lack of knowledge. I am using `q2` since the `ForEachAsync` method seems to return a `Task` that does not have the `ToListAsync` method available. — span, Feb 17 '18 at 20:55

score 1 · Accepted Answer · answered Feb 17 '18 at 22:46

Sorry about the comments, now with the model it looks clear to me what you are looking for. And you are right, the second statement would sort the Skills list.

Anyway, if you want to sort the child collection without risking calling twice to the database through your IQueryable, you can take first the list of courses asynchronously, and then sort the Skills in memory:

// Create the list with all courses with their skills
var coursesWithSkills= await db.Courses
    .Include(i => i.Skills)
    .ThenInclude(i => i.Skill)
    .ToListAsync();

// Order the skills for each course once data is in memory
foreach(x in coursesWithSkills)
{
    x.Skills = x.Skills.OrderBy(o => o.Weight)
                       .ThenBy(o => o.SkillId)
                       .ToList());
}

If you need that sorting part to not block the current thread, you should run it with a Task.Run rather than an async operation, as all the sorting work is CPU intensive and will be done in memory. But I wouldn't go for early optimization and I would leave the foreach block as it is until you see any performance issue.

This is nice but preferrably I would like the items to be ordered by the generated SQL-call if possible. I'll leave this question open for a little while before accepting the answer. Cheers. — span, Feb 19 '18 at 21:09

Simplify linq expression that orders children

1 Answers1