Change object property in nested list based on a keyword

Question

I have these classes:

public class BaseSearchFilter
{
    public int IssueCount { get; set; }
    public bool IsSelected { get; set; }
    public string FilterDescription { get; set; }
}

public class IssueTypeSearchFilter : BaseSearchFilter
{
    public StandardIssueTypeDTO IssueType { get; set; }
    public bool IsVisible { get; set; }
}

public class GroupedIssueTypeSearchFilter : ReactiveObject
{
    public int CategoryID { get; set; }
    public string CategoryTitle { get; set; }

    int selectionCode;
    public int SelectionCode
    {
        get { return selectionCode; }
        set { this.RaiseAndSetIfChanged(ref selectionCode, value); }
    }

    public ReactiveList<IssueTypeSearchFilter> IssueTypeFilterList { get; set; }
}

I also have AllGroupedStandardIssueTypesFilter which is a list of GroupedIssueTypeSearchFilter and the value will be something like this:

[0]:
  CategoryID = 1;
  CategoryTitle = "Test A";
  SelectionCode = 1;
  IssueTypeFilterList =
      [0]: 
         IssueType = //some value
         IsVisible = false;
         IssueCount = 23;
         IsSelected = false;
         FilterDescription = "some description"
      [1]: 
         IssueType = //some value
         IsVisible = false;
         IssueCount = 10;
         IsSelected = true;
         FilterDescription = "some description"
      [and so on...]
[1]:
  CategoryID = 3;
  CategoryTitle = "New Title";
  SelectionCode = 2;
  IssueTypeFilterList =
      [0]: 
         IssueType = //some value
         IsVisible = false;
         IssueCount = 4;
         IsSelected = false;
         FilterDescription = "some description"
      [1]: 
         IssueType = //some value
         IsVisible = false;
         IssueCount = 2;
         IsSelected = false;
         FilterDescription = "some description"
      [and so on...]
[and so on...]

I need to filter AllGroupedStandardIssueTypesFilter based on a keyword (string).

For example, if the keyword is "New", I need to find all objects in IssueTypeFilterList.IssueType that contain the keyword and change its IsVisible property to true and the rest to false.

This is the function that I created to filter AllGroupedStandardIssueTypesFilter:

FilterIssueType = ReactiveCommand.CreateAsyncTask(async x =>
{
    Func<IssueTypeSearchFilter, bool> filterFunc = d =>
        string.IsNullOrEmpty(this.IssueTypeSearchQuery) ? true :
        d.IssueType.Title.ToLower().Contains(this.IssueTypeSearchQuery.ToLower()) ||
        d.IssueType.CategoryTitle.ToLower().Contains(this.IssueTypeSearchQuery.ToLower()) ||
        d.IssueCount.ToString().Contains(this.IssueTypeSearchQuery);

    var filteredStdIssueTypes = AllStandardIssueTypesFilter.Where(filterFunc);

    Func<GroupedIssueTypeSearchFilter, bool> filterFunc1 = d =>
        string.IsNullOrEmpty(this.IssueTypeSearchQuery) ? true :
        d.IssueTypeFilterList.Any(a => a.IssueType.Title.ToLower().Contains(this.IssueTypeSearchQuery.ToLower())) ||
        d.IssueTypeFilterList.Any(b => b.IssueType.CategoryTitle.ToLower().Contains(this.IssueTypeSearchQuery.ToLower())) ||
        d.IssueTypeFilterList.Any(c => c.IssueCount.ToString().Contains(this.IssueTypeSearchQuery.ToLower()));

    var filteredGroupedStdIssueTypes = AllGroupedStandardIssueTypesFilter.Where(filterFunc1);

    for (int i = 0; i < filteredGroupedStdIssueTypes.Count(); i++)
    {
        var result = filteredStdIssueTypes.Where(dd => dd.IssueType.CategoryId == filteredGroupedStdIssueTypes.ElementAt(i).CategoryID);
        var notInFilteredResult = filteredGroupedStdIssueTypes.ElementAt(i).IssueTypeFilterList.Except(result).ToList();
        foreach(IssueTypeSearchFilter issue in notInFilteredResult)
        {
            issue.IsVisible = false;
        }
        var filteredResult = filteredGroupedStdIssueTypes.ElementAt(i).IssueTypeFilterList.Except(notInFilteredResult).ToList();
        foreach (IssueTypeSearchFilter issue in filteredResult)
        {
            issue.IsVisible = true;
        }
    }

    return await Task.FromResult(filteredGroupedStdIssueTypes);
});

Is there any improvement that I could do to that filter function?

Answer 1

Properly name everything: filterFunc and (even worse) filterFunc1 aren't informative at all.

---

Don't do this:

a.IssueType.Title.ToLower().Contains(this.IssueTypeSearchQuery.ToLower())

If you need a case insensitive Contains, write a helper method:

    private bool Contains(string source, string filter)
    {
        return source.IndexOf(filter, StringComparison.InvariantCultureIgnoreCase) >= 0;
    }

You could also turn this into an extension method, of course.

---

Note that your code isn't consistent:

d.IssueCount.ToString().Contains(this.IssueTypeSearchQuery);

vs:

c.IssueCount.ToString().Contains(this.IssueTypeSearchQuery.ToLower()));

Answer 2

FilterIssueType = ReactiveCommand.CreateAsyncTask(async x =>
{
     ....
}

You have big lambda here with even more nested lambdas. Consider refactoring this into separate filter methods that you can test and maintain independantly.

---

filteredStdIssueTypes.Where(dd => dd.IssueType.CategoryId == filteredGroupedStdIssueTypes.ElementAt(i).CategoryID);

You use ids here to get matching elements from the other collection so most probably a join like this would be more efficient and easier to read.

var joined = 
    from x in filteredStdIssueTypes
    join y in filteredGroupedStdIssueTypes on x.IssueType.CategoryId equals y.CategoryID
    select new { x, y };

---

In your loop you call

filteredStdIssueTypes.Where

on each iteration but you don't store the results of the previous filtering as a list or an array.

var filteredGroupedStdIssueTypes = AllGroupedStandardIssueTypesFilter.Where(filterFunc1); 

// no .ToList() or .ToArray()

this means that the filter runs over and over agian on each iteration. If there are a lot of items to filter it might be huge performance hit.