Better or optimized way to filter customer records by 3 dropdown filters

Question

I have one page name as:CustomerList.aspx on which i am displaying list of customers.

This is my table and class files too:

public partial class Customer
    {
        public int CustomerID { get; set; }
        public string FullName { get; set; }
        public string EmailId { get; set; }
        public int CustomerLocation { get; set; }
        public bool IsActive { get; set; }
        public bool Removed { get; set; }
        public DateTime SubscribeDate { get; set; }
        public Location _Location;
    }

    public partial class Location
    {
        public int LocationId { get; set; }
        public string Name { get; set; }
    }

Inactive=true:Means customer is active in the system.
Inactive=false:Means customer is inactive in the system.

Removed=true:Means customer is removed from the system

Removed=false:Means customer is not removed from the system.

I will provide user with 3 filters to filter customer records.

1)Location Dropdown

<select>
<option Text="All" Value="0" selected="true">
<option Text="London" Value="1">
<option Text="America" Value="2">
</select>

2)Status Dropdown with Value:All,Active,Inactive:

<select>
<option Text="All" Value="0" selected="true">
<option Text="Active" Value="1">
<option Text="Inactive" Value="2">
</select>

3)Stats Data Dropdown:

<select>
<option Text="All" Value="all" selected="true">
<option Text="Active Customers" Value="all">
<option Text="Recent subscribe customers" Value="subscribe">
<option Text="Recent unsubscribe customers" Value="unsubscribe">
</select>

As my page is loaded i want to display customers list in my grid.

This is my code:

 public void DisplayCustomersList()
        {
           DataTable list=GetCustomers(Convert.ToInt16(ddlLocation.SelectedValue),Convert.ToInt16(ddlStatus.SelectedValue),ddlstats.SelectedValue);
           Grid1.DataSource = list;
           Grid1.DataBind();
        }


  public DataTable GetCustomers(int LocationId, int ActiveId, string stats)
        {
            using (var context = new MyContext())
            {
                var data = from c in context.Customers
                           where c.Removed == false
                           select new
                           {
                               FullName = c.FullName,
                               c.CustomerID,
                               c._Location.Name,
                               c.IsActive,
                               c.SubscribeDate,
                               c.Removed
                           };
                if (LocationId != 0 && ActiveId != 0)
                {
                    if (ActiveId == 1)
                    {
                        return
                            MyContext.CopyToDataTable(
                                data.Where(x => x.LocationId == LocationId && x.IsActive == true && x.Removed == false));
                    }
                    else if(ActiveId==2)
                    {
                        return
                           MyContext.CopyToDataTable(
                               data.Where(x => x.LocationId == LocationId && x.IsActive == false && x.Removed == false));
                    }
                    return
                        MyContext.CopyToDataTable(
                            data.Where(x => x.LocationId == LocationId && x.Removed==false));
                }

                if (LocationId != 0 && stats != "")
                {
                    if (stats == "all")
                    {
                        return
                            MyContext.CopyToDataTable(
                                data.Where(x => x.LocationId == LocationId && x.IsActive == true && x.Removed == false));
                    }
                    else if (stats == "subscribe")
                    {
                        return
                           MyContext.CopyToDataTable(
                               data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.LocationId==LocationId));
                    }
                }

                if (ActiveId != 0 && stats != "")
                {
                    if(ActiveId==1)
                    {
                        if(stats=="all")
                        {
                            return
                            MyContext.CopyToDataTable(
                                data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == true) || (x.Removed == false) || (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false)));
                        }
                        else if (stats == "subscribe")
                        {
                            return
                               MyContext.CopyToDataTable(
                                   data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == true));
                        }
                        else if (stats == "unsubscribe")
                        {
                            return
                              MyContext.CopyToDataTable(
                                  data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));
                        }
                        
                    }
                    else if(ActiveId==2)
                    {
                        if (stats == "all")
                        {
                            MyContext.CopyToDataTable(
                                data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false) && (x.Removed == false)));
                        }
                        else if (stats == "subscribe")
                        {
                            return
                               MyContext.CopyToDataTable(
                                   data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));
                        }
                        else if (stats == "unsubscribe")
                        {
                            return
                              MyContext.CopyToDataTable(
                                  data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));
                        }
                    }
                }

                if (stats != "")
                {
                    if (stats == "all")
                    {
                        return
                            MyContext.CopyToDataTable(
                                data.Where(x => x.IsActive == true && x.Removed == false));
                    }
                    else if (stats == "subscribe")
                    {
                        return
                           MyContext.CopyToDataTable(
                               data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive==true));
                    }
                    else
                    {
                        return
                       MyContext.CopyToDataTable(
                           data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.IsActive == false && x.Removed == false));
                    }

                }
            }
        }

On all 3 dropdown selected index change event i am just calling this function like this:DisplayCustomersList()

So i just want to ask you that is this a proper way to perform filters or this code can be optimize in a better way.

can anybody provide me better solution or optimize this code in better way if possible???

Answer 1

This problem can be optimized without PredicateBuilder, but it requires careful and "systematic" analysis.

---

To begin with... Consider Your Predicate Making Determining Factor

Consider your case, putting all 14 of them together like this, you actually can see that you only have three determining factors, namely: LocationId, ActiveId, and stats:

No  LocationId  ActiveId    stats       result
1   not 0       1           don't care  data.Where(x => x.LocationId == LocationId && x.IsActive == true && x.Removed == false)
2   not 0       2           don't care  data.Where(x => x.LocationId == LocationId && x.IsActive == false && x.Removed == false));
3   not 0       not 0-2     don't care  data.Where(x => x.LocationId == LocationId && x.Removed == false));
4   not 0       don't care  all         data.Where(x => x.LocationId == LocationId && x.IsActive == true && x.Removed == false)
5   not 0       don't care  subscribe   data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.LocationId == LocationId));
6   don't care  1           all         data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == true) || (x.Removed == false) || (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false)));
7   don't care  1           subscribe   data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == true));
8   don't care  1           unsubscribe data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));
9   don't care  2           all         data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false) && (x.Removed == false)));
10  don't care  2           subscribe   data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));
11  don't care  2           unsubscribe data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));
12  don't care  don't care  all         data.Where(x => x.IsActive == true && x.Removed == false));
13  don't care  don't care  subscribe   data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == true));
14  don't care  don't care  unsubscribe data.Where(x => x.SubscribeDate >= DateTime.Now.AddDays(-7) && x.Removed == false && x.IsActive == false));

---

Next, consider the pattern of your results

I observe that your result is quite deterministic with small exceptions. Apart from result no 6 and no 9, your query predicates can actually be separated into four basic components (6 and 9 omitted). They are:

comp1: x.LocationId == LocationId
comp2: x.IsRemoved == false
comp3: x.IsActive == true
comp4: x.SubscribeDate >= DateTime.Now.AddDays(-7)

And the query logic is simply:

comp1 && comp2 && comp3 && comp4

Putting them together with the 12 cases (excluding the cases 6 and 9), you would get:

Simplification:
DC = don't care
A = applied
NA = not applied

                                        QueryComponents
No  LocationId  ActiveId    stats       comp1   comp2   comp3   comp4
1   not 0       1           DC          A       A       Yes     NA
2   not 0       2           DC          A       A       No      NA
3   not 0       not 0-2     DC          A       A       NA      NA
4   not 0       DC          all         A       A       Yes     NA
5   not 0       DC          subscribe   A       A       NA      A
7   DC          1           subscribe   NA      A       Yes     A
8   DC          1           unsubscribe NA      A       No      A
10  DC          2           subscribe   NA      A       No      A
11  DC          2           unsubscribe NA      A       No      A
12  DC          DC          all         NA      A       Yes     A
13  DC          DC          subscribe   NA      A       Yes     A
14  DC          DC          unsubscribe NA      A       No      A

---

After all the breaking down, we can see the Mapping

Now, it can be seen that the query components can be mapped back together with the determining factors:

comp1: Applied only when LocationId is not 0
comp2: Always applied //this is very good!
comp3: Yes = 1, 4, 7, 12, 13; NA = 3, 5; No = 2, 8, 10, 11, 14
comp4: Not Applied when LocationId is 0 except on case 5

---

You can start translating the concept into codes...

Thus, we can make some helping flags (there are 4 of them) to determine if a query component should be included or not, like this:

bool LocationIdNotApplied = LocationId == 0; //for comp1
bool IsActiveNotApplied = LocationId != 0 && (ActiveId < 0 || ActiveId > 2 || stats = "subscribe"); //for comp3 to be applied or not
bool IsActiveFalse = (LocationId != 0 && ActiveId == 2) || stats == "unsubscribe" || (ActiveId == 2 && stats == "subscribe"); //for comp3 to be false
bool DateApplied = LocationId == 0 || (LocationId != 0 && stats == "subscribe");

Then your data.Where for all cases except 6 and 9 can be simplified like this:

data.Where(x => (x.LocationId == LocationId || LocationIdNotApplied) //comp1
  && x.IsRemoved == false //comp2
  && ((x.IsActive == !IsActiveFalse) || IsActiveNotApplied) //comp3
  && (x.SubscribeDate >= DateTime.Now.AddDays(-7) || !DateApplied)) //comp4

This is a significant simplification for 12 cases becoming 1 case, and you only need to add the additional two cases, totalling in 3 cases rather than the original 14 cases!

---

Putting Them together into code

public DataTable GetCustomers(int LocationId, int ActiveId, string stats)
{
    using (var context = new MyContext())
    {
        var data = from c in context.Customers
                   where c.Removed == false
                   select new
                   {
                       FullName = c.FullName,
                       c.CustomerID,
                       c._Location.Name,
                       c.IsActive,
                       c.SubscribeDate,
                       c.Removed
                   };

        bool LocationIdNotApplied = LocationId == 0; //for comp1
        bool IsActiveNotApplied = LocationId != 0 && (ActiveId < 0 || ActiveId > 2 || stats = "subscribe"); //for comp3 to be applied or not
        bool IsActiveFalse = (LocationId != 0 && ActiveId == 2) || stats == "unsubscribe" || (ActiveId == 2 && stats == "subscribe"); //for comp3 to be false
        bool DateApplied = LocationId == 0 || (LocationId != 0 && stats == "subscribe");

        if(LocationId == 0 && ActiveId == 1 && stats == "all"){ //case 6
            return MyContext.CopyToDataTable(
                     data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == true) || (x.Removed == false) || (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false)));          
        } else if (LocationId == 0 && ActiveId == 2 && stats == "all"){ //case 9
            return MyContext.CopyToDataTable(
                     data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false) && (x.Removed == false)));
        } else { //other cases
            return MyContext.CopyToDataTable(
                     data.Where(x => (x.LocationId == LocationId || LocationIdNotApplied) //comp1
                       && x.IsRemoved == false //comp2
                       && ((x.IsActive == !IsActiveFalse) || IsActiveNotApplied) //comp3
                       && (x.SubscribeDate >= DateTime.Now.AddDays(-7) || !DateApplied))) //comp4
        }
    }        
}

---

Last Notes

Your case 6 actually seems strange to me:

data.Where(x => (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == true) || (x.Removed == false) || (x.SubscribeDate >= DateTime.Now.AddDays(-7) || x.IsActive == false)));

Note that you have both x.IsActive == true and x.IsActive == false for x.SubscribeDate >= DateTime.Now.AddDays(-7). And you combine it with ||. This is like saying:

(A || true) || (A || false)

And will always return true no matter what. You might want to check again and you may even simplify further/

---

Last Remarks and Apologize

Thus goes my solution for this case without PredicateBuilder - it requires careful and "systematic" (or, what I actually mean is step by step) analysis on all possible cases.

I have to apologize the OP because I cannot fully test the code which I have proposed for lacking of complete testing resources (unlike the OP).

But if the OP finds that there is a case which I miss to handle or is not put by the OP in the original question, at the very least, the steps presented in my solution above should still be useful for the OP to make his/her own careful analysis for his/her actual cases.

Answer 2

You can optimize search conditions in GetCustomer method. Please take a look Linqkit

Here basic example

http://www.albahari.com/nutshell/predicatebuilder.aspx https://www.nuget.org/packages/LinqKit/

Using PredicateBuilder

Here's how to solve the preceding example with PredicateBuilder:

IQueryable<Product> SearchProducts (params string[] keywords)
{
  var predicate = PredicateBuilder.False<Product>();

  foreach (string keyword in keywords)
  {
    string temp = keyword;
    predicate = predicate.Or (p => p.Description.Contains (temp));
  }
  return dataContext.Products.Where (predicate);
}