Amasuriel
Amasuriel

Reputation: 2200

Improve Reused on data filtering code

Anyone want to help make me a better programmer?

I have an application where I am filtering a collection of objects on a variety of criteria. Right now my code works, but 90% is duplicated which I feel like means I am doing something wrong. Any suggestions on how to make more reusable filtering code?

Assuming I have a collection of objects like this:

    public class ExampleData
{
    public int SomeValue1 { get; set; }
    public int SomeValue2 { get; set; }
    public int SomeValue3 { get; set; }
    public string SomeValue4 { get; set; }
}

And here is my filter class. Look at the code comments.

  class ExampleFilter
{
    public ExampleFilter()
    {
    }

    public IEnumerable<ExampleData> applyFilters(SearchCriteria criteria, IEnumerable<ExampleData> data)
    {
        //The body of these methods is almost identical...how can I better design this process so I don't cut and paste 90% of the code?
        data = filterByValue1Selections(criteria.FormTemplateSelected, data);
        data = filterByValue2Selections(criteria.FormTemplateSelected, data);
        return data;
    }

    public IEnumerable<ExampleData> filterByValue1Selections(List<int> value1Ids, IEnumerable<ExampleData> data)
    {
        if (value1Ids != null)
        {
            IEnumerable<ExampleData> dataQuery = null;
            foreach (int selectedValueIds in value1Ids)
            {
                //See http://justgeeks.blogspot.com/2011/01/using-linq-in-foreach-loop-to-build.html
                //For explanation of why we copy this locally
                int selectedId = selectedValueIds;
                if (dataQuery == null)
                {
                    //This code and the similar block in the else are the only differences in these methods
                    dataQuery = data.Where(t => t.SomeValue1 == selectedId);
                }
                else
                {
                    dataQuery = dataQuery.Union(data.Where(t => t.SomeValue1 == selectedId));
                }
            }
            data = dataQuery;
        }
        return data;
    }

    public IEnumerable<ExampleData> filterByValue2Selections(List<int> value2Ids, IEnumerable<ExampleData> data)
    {
        if (value2Ids != null)
        {
            IEnumerable<ExampleData> dataQuery = null;
            foreach (int selectedValueIds in value2Ids)
            {
                //See http://justgeeks.blogspot.com/2011/01/using-linq-in-foreach-loop-to-build.html
                //For explanation of why we copy this locally
                int selectedId = selectedValueIds;
                if (dataQuery == null)
                {
                    dataQuery = data.Where(t => t.SomeValue1 == selectedId);
                }
                else
                {
                    dataQuery = dataQuery.Union(data.Where(t => t.SomeValue1 == selectedId));
                }
            }
            data = dataQuery;
        }
        return data;
    }


}

Upvotes: 0

Views: 89

Answers (2)

MonkeyCoder
MonkeyCoder

Reputation: 2620

In general you could make use of Func<T, TResult> delegates to generalize the repeated FilterByValue methods, you could try something like this (pseudo-code):

public IEnumerable<T> FilterByValue<T>(List<int> value1Ids, IEnumerable<T> data, Func<T, int> selector)
{
    if (value1Ids != null)
    {
        IEnumerable<ExampleData> dataQuery = null;
        foreach (int id in value1Ids)
        {
            int selectedId = id;
            if (dataQuery == null)
            {
                dataQuery = data.Where(x => selector(x) == id);
            }
            else
            {
                dataQuery = dataQuery.Union(data.Where(x => selector(x) == id));
            }
        }

        data = dataQuery;
    }

    return data;
}

This way you can pass the actual property selection:

FilterByValue(criteria.FormTemplateSelected, data, x => SomeValue1);
FilterByValue(criteria.FormTemplateSelected, data, x => SomeValue2);

Depending on the scenario you can go even further and apply more complex logic on the delegate but it's hard to judge from the code you've pasted which part you are interested the most.

Upvotes: 1

BrokenGlass
BrokenGlass

Reputation: 160942

You are basically re-implementing the Contains() method - you can just do:

List<int> valueIds = ...;
var filteredData = data.Where(x=> valueIds.Contains(x.SomeValue1));

Then your applyFilters method would look like this:

public IEnumerable<ExampleData> applyFilters(SearchCriteria criteria, IEnumerable<ExampleData> data)
{
    data = data.Where(x=> criteria.FormTemplateSelected.Contains(x.SomeValue1)
                       && criteria.FormTemplateSelected.Contains(x.SomeValue2));
    return data;
}

Upvotes: 2

Related Questions