CODEWITHSUNDEEP
c#.netlinqconditional-statements
pointlesspolitics
pointlesspolitics

Reputation: 2480

Simplifying complex linq query with multiple conditions

Recently I have come across the code to fix a bug as shown below. It is just hard to understand it and make the changes seem risky as well. Is there any easy way to breakdown such a query i.e. De Morgan's law or re-writing in different way. 

SomeReturnType xyz() 
{
    return (from m in this.context.tblMessages
            where m.SystemActionID.HasValue &&
            (userID == null || m.RecipientID == null || m.RecipientID == userID) &&
            m.TargetUserType == userType &&
            (
                 (territoryID.HasValue && !m.TerritoryID.HasValue) ||
                 (!territoryID.HasValue && !m.TerritoryID.HasValue) ||
                 (territoryID.HasValue && m.TerritoryID.HasValue && territoryID.Value == m.TerritoryID.Value)
            ) &&
            (
                  (regionID.HasValue && !m.RegionID.HasValue) ||
                  (!regionID.HasValue && !m.RegionID.HasValue) ||
                  (regionID.HasValue && m.RegionID.HasValue && regionID.Value == m.RegionID.Value)
            ) &&
            (
                  (teamID.HasValue && !m.TeamID.HasValue) ||
                  (!teamID.HasValue && !m.TeamID.HasValue) ||
                  (teamID.HasValue && m.TeamID.HasValue && teamID.Value == m.TeamID.Value)
            ) &&
            m.SystemActionData == additionalData &&
            (!completed.HasValue || m.IsSystemActionCompleted == completed.Value)
            select m).FirstOrDefault();
}  //function xyz ends

Upvotes: 0

Views: 1102

Answers (2)

Maxim
Maxim

Reputation: 13458

I am using IQueryable in such cases. I have used some dummy names instead of your example to demonstrate idea itself - you need to apply "where" clauses step by step.

IQueryable<Account> query1 = from account in storage.Accounts
                                where account.Username == username
                                select account;

IQueryable<SomeNewTypeIfNecessary> query2 = from account in query1
                                            where account.ID > 100
                                            select new SomeNewTypeIfNecessary { ID = account.ID };

// Final call doing real query to database.
List<SomeNewTypeIfNecessary> accounts = query2.ToList();

In addition such technique allows to dynamically add desired statements (mostly where and orderby clauses) to get specific data dependent on some user choice (for example sort direction).

Upvotes: 1

Panagiotis Kanavos
Panagiotis Kanavos

Reputation: 131423

First, this code looks like an attempt to move bad SQL code to LINQ: the statements contain parameter checks in an attempt to create a query that can have "optional" parameters. It looks like the author tried to move a stored procedure to code.

Both of these things are really bad ideas.

  • First, SQL Server will cache a query's execution plan according to its parameters. This means that the first execution of the procedure determines the plan for all later calls. "Optional" parameters typically result in non-optimal plans (eg by omitting indexes)
  • Stored procedures are an abstraction layer that hides complex queries, allows you to use the full SQL syntax of the product. Moving them to code just makes things harder, not easier.
  • Finally, LINQ doesn't need "optional" parameters. You can compose your query and the final SQL statement will be generated when you convert the IQueryable to an IEnumerable. In this case, when you call FirstOrDefault.

For example, you can change this:

(from m in this.context.tblMessages
        where m.SystemActionID.HasValue &&
        (userID == null || m.RecipientID == null || m.RecipientID == userID) &&
        m.TargetUserType == userType &&
        (
             (territoryID.HasValue && !m.TerritoryID.HasValue) ||
             (!territoryID.HasValue && !m.TerritoryID.HasValue) ||
             (territoryID.HasValue && m.TerritoryID.HasValue && territoryID.Value == m.TerritoryID.Value)
        ) &&

to this

if (userId==null)
    return null;
var query=from m in this.context.tblMessages
        where m.SystemActionID != null 
              && (m.RecipientID == null || m.RecipientID == userID)
              && m.TargetUserType == userType ;
if (territoryID.HasValue)
    query=query.Where(m=>m.TerritoryId==null || m.TerritoryId==teritoryID);

You can simplify the rest of the statement in the same way, eg: 

if (regionID.HasValue)
    query=query.Where(m=>m.RegionId==null || m.RegionId==regionID);
if (teamID.HasValue)
    query=query.Where(m=>m.TeamID==null || m.TeamID==teamID);

The LINQ query should never compare parameters against constants - why force the server to do a constantc check you can easily do on the client?

This will result in queries that are a lot simpler and easier for the server to optimize

Upvotes: 3

Related Questions