dotnet / dotnet/roslynator

Suggestion: LINQ DB analyzers & Fixes

Open
#430 0 comments 0 reactions 0 assignees View on GitHub
Area-Analyzers Feature Request
Dominant language
C#
Stars
3.5k
Forks
294
Avg merge
2h 30m
Merged PRs (30d)
4

Description

I was recently working with some code that parameterized a query in an arguably over-clever way. The SQL it generated was bloated and complex.

This approximates the "BEFORE" code:
```csharp
private IEnumerable> GetClientInfo(DbContext db)
{
string singleItem = String.IsNullOrWhiteSpace(options.SingleItem) ? String.Empty : options.SingleItem.Trim();

return db.ClientInfo
.Where(
p => p.DateSent == null
&& (!options.ClientList.Any() || options.ClientList.Contains(p.ClientId))
&& (!options.ExcludedClientsList.Any() || !options.ExcludedClientsList.Contains(p.ClientId))
&& (String.IsNullOrEmpty(singleItem) || p.ItemLocator.Equals(singleItem, StringComparison.OrdinalIgnoreCase))
&& (p.SendAfter == null || p.SendAfter < DateTime.Now)
)
.OrderBy(p => p.DateAdded)
.GroupBy(p => p.ClientId).ToList();
}
```
I changed the code by introducing a local for the query and then breaking out the `&&`'s into additional chained `Where` calls, each guarded with an appropriate condition. The AFTER code is a little longer, but the resulting SQL is simpler because of the parts that are no longer necessary when the conditions are right.

The "AFTER" code looks something like this:
```csharp
private IEnumerable> GetClientInfo(DbContext db)
{
string singleItem = String.IsNullOrWhiteSpace(options.SingleItem) ? String.Empty : options.SingleItem.Trim();

IQueryable query =
db.ClientInfo
.Where(p => p.DateSent == null && (p.SendAfter == null || p.SendAfter < DateTime.Now));

if (options.ClientList.Any())
{
query = query.Where(p => options.ClientList.Contains(p.ClientId));
}

if (options.ExcludedClientsList.Any())
{
query = query.Where(p => !options.ExcludedClientsList.Contains(p.ClientId));
}

if (!String.IsNullOrEmpty(singleItem))
{
query = query.Where(p => p.ItemLocator == singleItem); // Db string comparison is already case-insensitive
}

return query
.OrderBy(p => p.DateAdded)
.GroupBy(p => p.ClientId)
.ToList();
}
```
The options.ClientList and options.ExcludedClientsList are properties that are in-memory collections. For the purpose of the query, the "singleItem" local is simple static text. With the lazy nature of IQueryable queries, that `.ToList()` at the end may make this case unique. Also, I haven't discounted the possibility that the list properties could be `null`, in which case my conditionals might need to change accordingly. This could also be an incredibly narrow analyzer niche to fill.

Regardless, even though I can intuit the pattern and manually apply it here, it would be really nice to have automated tool to break it apart like this. This request could be a forerunner of a whole family of LINQ related analyzers and refactorings based on interactions between `IEnumerable` and `IQueryable`. Interesting?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.