c# - How to make this EF Linq method syntax query more readable - Stack Overflow

Can I somehow make this query more readable? I don't like ternary operators inside the select.

var chats = await context.Chats
        .Where(x => x.Name.Contains(query))
        .Include(x => x.UserChats)
        .Include(x => x.Messages)
            .ThenInclude(x => x.User)
        .Select(x => new ChatResponse(
            x.Id,
            x.Name,
            x.Description,
            x.ChatType,
            x.UserChats.Count,
            x.UserChats.Any(uc => uc.UserId == httpContextProvider.CurrentUserId),
            x.Messages.LastOrDefault() == null ? String.Empty : x.Messages.LastOrDefault()!.Content,
            x.Messages.LastOrDefault() == null ? null : x.Messages.LastOrDefault()!.CreatedAt,
            x.Messages.LastOrDefault() == null ? String.Empty : x.Messages.LastOrDefault()!.User.UserName!
        )).ToListAsync();

Can I somehow make this query more readable? I don't like ternary operators inside the select.

var chats = await context.Chats
        .Where(x => x.Name.Contains(query))
        .Include(x => x.UserChats)
        .Include(x => x.Messages)
            .ThenInclude(x => x.User)
        .Select(x => new ChatResponse(
            x.Id,
            x.Name,
            x.Description,
            x.ChatType,
            x.UserChats.Count,
            x.UserChats.Any(uc => uc.UserId == httpContextProvider.CurrentUserId),
            x.Messages.LastOrDefault() == null ? String.Empty : x.Messages.LastOrDefault()!.Content,
            x.Messages.LastOrDefault() == null ? null : x.Messages.LastOrDefault()!.CreatedAt,
            x.Messages.LastOrDefault() == null ? String.Empty : x.Messages.LastOrDefault()!.User.UserName!
        )).ToListAsync();

• c#
• entity-framework
• linq

Share Improve this question Follow edited Jan 22 at 21:16 jarlh 44.8k88 gold badges5050 silver badges6767 bronze badges asked Jan 22 at 20:56 IllusionIllusion 3355 bronze badges 4

• I removed the <sql> tag since this isn't really an SQL language question. Perhaps you want to add <sql-server> instead? – jarlh Commented Jan 22 at 21:17
• 1 For one, remove the Includes. They're not necessary. – Gert Arnold Commented Jan 22 at 21:18
• You may be able to use the null propagation ?. and the coalesce ?? operators - x.Messages.LastOrDefault()?.Content ?? String.Empty. Slight difference in behavior is when the actual referenced values are ever null, those will also be replaced with String.Empty. (Might be a good thing.) – T N Commented Jan 22 at 22:54
• Side note: Without an .OrderBy() operation, x.Messages.LastOrDefault() could return any arbitrary message, not just the latest one. Although this may appear to work on a small scale in a test environment, the results are not guaranteed and may randomly break when scaled up in production. Better to use x.Messages.OrderBy(m => m.CreatedAt).LastOrDefault() or x.Messages.OrderByDescending(m => m.CreatedAt).FirstOrDefault(). (I am not sure whether or not the generated SQL can be optimized for the .LastOrDefault() case.) – T N Commented Jan 22 at 23:42

Add a comment  | 

2 Answers 2

Sorted by: Reset to default 2

I would suggest a double-projection. With projection you do not need eager loading, so no need for Include. Double-projection first uses a pure Linq-to-entity query to get the relevant data then a second projection to flatten to the desired view model.

var chats = await context.Chats
    .Where(x => x.Name.Contains(query))
    .Select(x => new 
    {
        x.Id,
        x.Name,
        x.Description,
        x.ChatType,
        ChatCount = x.UserChats.Count,
        CurrentUserInChat = x.UserChats.Any(uc => uc.UserId == httpContextProvider.CurrentUserId),
        LastMessage = x.Messages
            .OrderByDescending(m => m.CreatedAt)
            .Select(m => new 
            {   
                m.CreatedAt,
                m.Content,
                m.User.UserName
            }).FirstOrDefault()
    }).ToListAsync() // First projection to SQL
    .Select(x => new ChatResponse(
            x.Id,
            x.Name,
            x.Description,
            x.ChatType,
            x.ChatCount,
            x.CurrentUserInChat,
            x.LastMessage?.Content ?? string.Empty,
            x.LastMessage?.CreatedAt,
            x.LastMessage?.UserName ?? string.Empty)
     .ToList(); // Second projection in memory to model.

While it is possible to build a single query for complex queries that flatten information, these can sometimes end up being harder to read and can lead to less efficient queries where tables are joined multiple times for things like multiple FirstOrDefault() sub-queries. The double projection allows it to produce a single structured query (using OrderByDescending and FirstOrDefault rather than LastOrDefault) then a simple second pass to build the desired flattened projection. Some null coalesce operations like ?. don't work in Linq-to-Entity when building SQL, so we leave that for the second projection done in memory.

I recommend not limiting yourself to method syntax but instead focusing on creating simple and efficient queries. The following query is straightforward, easy to read, and performs about five times faster:

var chats = (
    from x in context.Chats
    where x.Name.Contains(query)
    from lastMessage in x.Messages
        .OrderByDescending(m => m.CreatedAt)
        .Take(1)
        .DefaultIfEmpty()
    select new ChatResponse(
        x.Id,
        x.Name,
        x.Description,
        x.ChatType,
        x.UserChats.Count(),
        x.UserChats.Any(uc => uc.UserId == httpContextProvider.CurrentUserId),
        lastMessage.Content ?? string.Empty,
        (DateTime?)lastMessage.CreatedAt,
        lastMessage.User.UserName ?? string.Empty
    )).ToListAsync();