Reputation: 2200
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
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
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