Tukukkk

I have the following code. Should the linq statements be combined or left as is?
I typically like to break it up but some members on my team like a one-liner.

Thoughts?

        var startDate = DateTime.UtcNow;

        var endDate = startDate.AddDays(expirationDays);



        var allTokens = await repository.Query<Domain.Models.AccessToken>()

            .Where(x => !x.IsRevoked)

            .ToListAsync()

            .ConfigureAwait(false);



        var filteredTokens = allTokens.GroupBy(x => x.ApplicationName)

            .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

            .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate);



        return filteredTokens.Select(x => x.Id).ToList();

Personally, I follow the mantra, "Programs must be written for people to read, and only incidentally for machines to execute" (Harold Abelson.) Make the code as readable as possible (and only then worry about optimizations/efficiency/etc if needed). It's easier to optimize clean code than it is to clean up optimized code.

So on that important readability front, I'd say: Break them Up! Breaking up your LINQ statements into multiple lines lets you do something very very valuable:

var records = someLinqStatement;

var activeUserRecords = records.SomeLinqStatement;

var mostRecentUserRecords = activeUserRecords.SomeLinqStatement;

... it lets you use the variable name to document the contents of the linq. You're using it to improve the readability of your code. If anything, you've grouped too much (there's a lot going on in that 3-line filteredTokens line in your version.)

Trying to read what your coworkers would have:

var allTokens = await repository.Query<Domain.Models.AccessToken>()

        .Where(x => !x.IsRevoked)

        .ToListAsync()

        .ConfigureAwait(false)

        .GroupBy(x => x.ApplicationName)

        .Select(y => y.OrderByDescending(z => z.ExpirationDate).FirstOrDefault())

        .Where(x => x != null && ! x.IsRevoked && x.ExpirationDate >= startDate && x.ExpirationDate <= endDate)

        .Select(x => x.Id).ToList();

... is a freaking nightmare. At that point, I'm no longer reading code, I'm slowly trying to decipher it.