How to Simplify Complex LINQ Expressions

Today I was writing a new repository for a new entity on my U413 project. I got to one method where I needed to write a somewhat messy lambda expression. In the past I've just written it and moved on but I decided to take a break and figure out a cleaner way to write these expressions. Here is the original method:

/// <summary>
/// Retrieves the total number of messages for the user.
/// </summary>
/// <param name="username">The name of the user.</param>
/// <param name="sent">True if retrieving the number of messages sent.</param>
/// <returns>The total number of messages.</returns>
public int GetMessageCountBy_Username(string username, bool sent)  
{
    var query = _dataContext.Messages
        .Where(x => ((sent ? x.Sender.ToLower() : x.Recipient.ToLower()) == username.ToLower()) && (sent ? !x.SenderDeleted : !x.RecipientDeleted))
        .Count();
    return query;
}

It's really not that bad, but the .Where() expression is kind of painful to read and parse. The obvious quick fix was to refactor the query like this:

/// <summary>
/// Retrieves the total number of messages for the user.
/// </summary>
/// <param name="username">The name of the user.</param>
/// <param name="sent">True if retrieving the number of messages sent.</param>
/// <returns>The total number of messages.</returns>
public int GetMessageCountBy_Username(string username, bool sent)  
{
    var query = _dataContext.Messages
        .Where(x => (sent ? x.Sender.ToLower() : x.Recipient.ToLower()) == username.ToLower())
        .Where(x => sent ? !x.SenderDeleted : !x.RecipientDeleted);
        .Count();
    return query;
}

That's a little better but it's still kind of a pain to look at. I like my queries to be as English readable as possible. I did some hunting and I found out how to declare an expression elsewhere so that it could be used more than once. Because I'm evaluating two separate pieces of logic I decided to make two expressions, like so:

/// <summary>
/// Expression to determine if a message belongs to a user.
/// </summary>
/// <param name="username">The name of the user.</param>
/// <param name="sent">True if retrieving sent messages.</param>
/// <returns>An expression to be used in a LINQ query.</returns>
private Expression<Func<Message, bool>> MessageBelongsToUser(string username, bool sent)  
{
    return x => (sent ? x.FromUser : x.ToUser).Username.Equals(username, StringComparison.OrdinalIgnoreCase);
}

/// <summary>
/// Expression to determine if a message has been deleted by the user.
/// </summary>
/// <param name="username">The name of the user.</param>
/// <param name="sent">True if retrieving sent messages.</param>
/// <returns>An expression to be used in a LINQ query.</returns>
private Expression<Func<Message, bool>> UserDidNotDeleteMessage(string username, bool sent)  
{
    return x => sent ? !x.SenderDeleted : !x.RecipientDeleted;
}

This is great, now imagine how readable those queries will be (or don't imagine and just look at them here :P):

/// <summary>
/// Retrieves a list of messages from the data context for a user.
/// </summary>
/// <param name="username">The name of the user.</param>
/// <param name="page">The page number.</param>
/// <param name="itemsPerPage">The number of items to display per page.</param>
/// <param name="sent">True if retrieving sent messages.</param>
/// <returns>An enumerable list of messages.</returns>
public IEnumerable<Message> GetMessagesBy_Username(string username, int page, int itemsPerPage, bool sent)  
{
    var query = _dataContext.Messages
        .Where(MessageBelongsToUser(username, sent))
        .Where(UserDidNotDeleteMessage(username, sent))
        .OrderByDescending(x => x.SentDate)
        .Skip(itemsPerPage * (page - 1))
        .Take(itemsPerPage);
    return query;
}

/// <summary>
/// Retrieves the total number of messages for the user.
/// </summary>
/// <param name="username">The name of the user.</param>
/// <param name="sent">True if retrieving the number of messages sent.</param>
/// <returns>The total number of messages.</returns>
public int GetMessageCountBy_Username(string username, bool sent)  
{
    var query = _dataContext.Messages
        .Where(MessageBelongsToUser(username, sent))
        .Where(UserDidNotDeleteMessage(username, sent))
        .Count();
    return query;
}

Notice how readable that is? "Where message belongs to user". It's perfect! On top of that, you'll notice I have two methods using these expressions. Before I would have had to duplicate those original lambda expressions in both methods. I don't know why I didn't learn about this sooner! Some simple refactoring of code can sometimes make all the difference in the world!

Aside: If you notice, in my MessageBelongsToUser expression I use Username.Equals(username, StringComparison.OrdinalIgnoreCase). During my search I found out that this is the proper way to do string comparisons. Apparently there are edge cases where .ToUpper() and .ToLower() as well as == do not compare string values correctly. I bring this up because I also wanted to mention that there are two ways to use the Equals() method and one of them does not work in an expression. The first way is to use it like I have in the example, as a method on the instance of an existing string variable. The way (that doesn't work in an expression) is to call the method on the static string class. string.Equals(str1, str2, StringComparison.IgnorCase) does not work because an expression can't make calls to outer classes, but extension/instanced methods seem to work just fine.

Chev

Read more posts by this author.

comments powered by Disqus