Simplify querying SQL in Entity Framework Core

Question

I have the following code:

public async Task<IEnumerable<Submission>> SelectSubmissionsAsync(string submitterId, IEnumerable<Group> groups)
{
    var submissions = new List<Submission>();

    var apps = context.Apps
                      .Select(a => new
                                   {
                                       Id = a.Id,
                                       Member = a.MemberHistories.OrderByDescending(ash => ash.MemberChangeDate).FirstOrDefault().Member,
                                       Owner = a.OwnerHistories.OrderByDescending(oh => oh.OwnerChangeDate).FirstOrDefault().Owner
                                   }) 
                      .ToDictionary(x => x.Id, x => x.Member + x.Owner);

    var subs = context.Submissions.ToList();

    foreach (var sub in subs)
    {
        if (apps.ContainsKey((Guid)sub.AppId))
        {
            var value = apps[(Guid)sub.AppId];
            var check = value.Contains(submitterId, StringComparison.InvariantCultureIgnoreCase) || groups.Any(g => value.Contains(g.Id, StringComparison.InvariantCultureIgnoreCase));

            if (check) 
                submissions.Add(sub);
        }
    }
}


public class Submission
{
    public Guid Id { get; set; }       
    public Application App { get; set; }
    public Guid? AppId { get; set; }
}

public class App
{
    public Guid Id { get; set; }
    public string Identifier { get; set; }        
    public ICollection<MemberHistory> MemberHistories { get; set;}
    public ICollection<OwnerHistory> OwnerHistories { get; set;}
}

Is there a way to simplify this code (avoid for loop for example)?

I doubt the code you have is actually what you want. If an app's member is "xyz" and their owner is "abc", and the group ID is "za", would you really want that app to match? Also, why does Submission's AppId need to be cast as a Guid? If it's a Guid coming out of the database, why not have that as it's declared type? — StriplingWarrior, Jul 15 '22 at 20:54
When you say `App.ContainsKey(...)` is that referring to a dictionary you've built somewhere else, or is that a typo and you really mean `apps.ContainsKey(...)`? Is there a relationship between `Submissions` and `Apps` in your EF context? — StriplingWarrior, Jul 15 '22 at 20:57
You shouldn’t get two tables completely and then write some loops and conditions. Try writing this in a single sql statement. Approach it from that domain you might see it. — Wouter, Jul 15 '22 at 21:10

score 0 · Answer 1 · answered Jul 15 '22 at 22:20

Ideally you should be able to construct a single query looking something like this:

var appInfo = context.Apps
    .Select(a => new
        {
            Id = a.Id,
            Member = a.MemberHistories.OrderByDescending(ash => ash.MemberChangeDate).FirstOrDefault().Member,
            Owner = a.OwnerHistories.OrderByDescending(oh => oh.OwnerChangeDate).FirstOrDefault().Owner
        })
    .Where(appCriteria)
;
var submissions = context.Submissions
    .Where(s => appInfo.Any(app => s.AppId == app.Id))
    .ToList();

That will allow your app to build a single SQL command that filters the apps down to just the ones you want before bringing them back from the database.

Building checkCriteria will be complicated, because that's going to be based on the "OR"/Union of several criteria. You'll probably want to build a collection of those criteria, and then combine them using a strategy similar to what I've defined here. If you start with a collection of values including submitterId and groupIds, each criteria would be something like s => s.Member == val || s.Owner == val.

In order to create these expressions, you'll probably need to declare a class to represent the type that you're currently using an anonymous type for, so you have a name to associate with the generic arguments on your Expression types.

Simplify querying SQL in Entity Framework Core

1 Answers1

Linked