1

I'm trying to optimize the following routine so that it only uses one database query instead of multiple but the filter (loop conditions) don't seem to be applied as all messeges are fetched.

     public async Task<List<Message>> GetInboxMessegesFromUser(string inboxUser, List<string> senders)
     {
             // Doesn't work

            var query = from m in _context.Messeges where m.ToUser == inboxUser select m;
            foreach (string sender in senders)
            {
                query.Where(m => m.FromUser == sender);
            }
            return await query.ToListAsync();

            // Old code 

            List<Message> messeges = new List<Message>();
            foreach (string sender in senders)
            {
                messeges.AddRange(await _context.Messeges.Where(m => m.FromUser == sender && m.ToUser == inboxUser).ToListAsync());
            }


               return messeges;
            }

How to add a dynamic number of conditions to a LINQ query before executing it so that a O(N*M) turns into O(N) ?

Tiago Redaelli
  • 560
  • 5
  • 17
  • 1
    replace: `query = query.Where(m => m.FromUser == sender);` – Rovann Linhalis Oct 17 '19 at 14:28
  • Also, this will be more effecient in your scenario instead of the foreach: https://stackoverflow.com/questions/10667675/linq-where-list-contains-any-in-list – Chris Dixon Oct 17 '19 at 14:29
  • 1
    That query will not produce any results due to a logic error even after you do the assignment `query = query.Where(....);`. You cant have a record that has more than 1 value on a property. So if you pass in 2 values in the `senders` List any one record will never be both of those values. The result is that this method will always return an empty list. – Igor Oct 17 '19 at 14:31

2 Answers2

2

That query will not produce any results due to a logic error even after you do the assignment query = query.Where(....);. You cant have a record that has more than 1 value on a property. So if you pass in 2 values in the senders List any one record will never be both of those values. The result is that this method will always return an empty list.

You can use .Contains to see if there is at least one match from a list which would translate to an IN clause in Sql Server.

Your whole method could be rewritten like this:

public Task<List<Message>> GetInboxMessegesFromUser(string inboxUser, List<string> senders)
{
    return _context.Messeges
        .Where(message => message.ToUser == inboxUser && senders.Contains(message.FromUser))
        .ToListAsync();
}
Igor
  • 60,821
  • 10
  • 100
  • 175
  • This is correct, but I have a hard time condoning using underscore as a variable name. – StriplingWarrior Oct 17 '19 at 14:40
  • 1
    To elaborate: underscores were arguably a bad variable name regardless, but with C# 7, Discards reinforced the notion that an underscore is a placeholder for something you don't actually intend to use. See https://learn.microsoft.com/en-us/dotnet/csharp/discards – StriplingWarrior Oct 17 '19 at 14:45
1

Try changing the existing code to:

messeges.AddRange(await _context.Messeges.Where(m => senders.Contains(m.FromUser) && m.ToUser == inboxUser).ToListAsync());
Matt.G
  • 3,586
  • 2
  • 10
  • 23