Expression<Func<ProductEntity,bool>> predicate = (p => (search.CategoryId ?? p.CategoryId) == p.CategoryId));
var q2 = q.Where(predicate);
Another possibility would be to dynamically compose a query predicate using PredicateBuilder. That's the way I do it for searches with a similar pattern as you use:
var predicate = PredicateBuilder.True<Order>();
if (search.OrderId))
{
predicate = predicate.And(a => SqlMethods.Like(a.OrderID, search.OderID);
}
// ...
var results = q.Where(predicate);
Expression<Func<ProductEntity,bool> predicate = p => !search.CategoryId.HasValue
|| (search.CategoryId.HasValue && search.CategoryId == p.CategoryId)
var q2 = q.Where(predicate);
refactor code
var predicate = PredicateBuilder.True<Country>();
if (!string.IsNullOrWhiteSpace(searchAlpha2))
predicate = predicate.And(c => c.Alpha2 != null ? c.Alpha2.Contains(searchAlpha2) : false);
if (!string.IsNullOrWhiteSpace(searchAlpha3))
predicate = predicate.And(c => c.Alpha3 != null ? c.Alpha3.Contains(searchAlpha3) : false);
if (!string.IsNullOrWhiteSpace(searchName))
predicate = predicate.And(c => c.Name != null ? c.Name.Contains(searchName) : false);
IQueryable<Country> countries = db.Countries.AsExpandable().Where(predicate);
Variables searchAlpha2, searchAlpha3 and searchName are strings defined by the user. The code filters a table for where column Alpha2 contains the text defined in searchAlpha2, column Alpha3 contains searchAlpha3, and the same again for Name & searchName.
A lot of the above code is repeated; I'd ideally like to refactor it to something like below:
var helper = new FilterHelper<Country>();
helper.AddContainsCondition(searchAlpha2, 'Alpha2');
helper.AddContainsCondition(searchAlpha3, 'Alpha3');
helper.AddContainsCondition(searchName, 'Name');
IQueryable<Country> countries = db.Countries.AsExpandable().Where(helper.Predicate);
public class FilterHelper<T>
{
private System.Linq.Expressions.Expression<Func<T,bool>> predicate;
public FilterHelper()
{
this.predicate = PredicateBuilder.True<T>();
}
public void AddContainsCondition(string searchText, string fieldName)
{
if (!string.IsNullOrWhiteSpace(searchText))
this.predicate = this.predicate.And(x => x.GetColumn(fieldName) != null ? x.GetColumn(fieldName).Contains(searchText) : false);
}
public System.Linq.Expressions.Expression<Func<T,bool>> Predicate
{
get{ return this.predicate; }
}
}
I currently have the below code:
var predicate = PredicateBuilder.True<Country>();
if (!string.IsNullOrWhiteSpace(searchAlpha2))
predicate = predicate.And(c => c.Alpha2 != null ? c.Alpha2.Contains(searchAlpha2) : false);
if (!string.IsNullOrWhiteSpace(searchAlpha3))
predicate = predicate.And(c => c.Alpha3 != null ? c.Alpha3.Contains(searchAlpha3) : false);
if (!string.IsNullOrWhiteSpace(searchName))
predicate = predicate.And(c => c.Name != null ? c.Name.Contains(searchName) : false);
IQueryable<Country> countries = db.Countries.AsExpandable().Where(predicate);
Variables searchAlpha2, searchAlpha3 and searchName are strings defined by the user. The code filters a table for where column Alpha2 contains the text defined in searchAlpha2, column Alpha3 contains searchAlpha3, and the same again for Name & searchName.
A lot of the above code is repeated; I'd ideally like to refactor it to something like below:
var helper = new FilterHelper<Country>();
helper.AddContainsCondition(searchAlpha2, 'Alpha2');
helper.AddContainsCondition(searchAlpha3, 'Alpha3');
helper.AddContainsCondition(searchName, 'Name');
IQueryable<Country> countries = db.Countries.AsExpandable().Where(helper.Predicate);
...
public class FilterHelper<T>
{
private System.Linq.Expressions.Expression<Func<T,bool>> predicate;
public FilterHelper()
{
this.predicate = PredicateBuilder.True<T>();
}
public void AddContainsCondition(string searchText, string fieldName)
{
if (!string.IsNullOrWhiteSpace(searchText))
this.predicate = this.predicate.And(x => x.GetColumn(fieldName) != null ? x.GetColumn(fieldName).Contains(searchText) : false);
}
public System.Linq.Expressions.Expression<Func<T,bool>> Predicate
{
get{ return this.predicate; }
}
}
However I can't think of a way to do this that makes sense (i.e. GetColumn is a made up function for illustrative purposes; to my knowledge nothing like this exists in the framework, besides which passing column names as strings would also lose the benefit of the compiler checking that these column names are valid). Is there a nice solution?
Update:
I found a potential workaround for my original issue, but this now gives me the error Unable to cast object of type 'System.Linq.Expressions.FieldExpression' to type 'System.Linq.Expressions.LambdaExpression'.
NB: I'm using the third party library LinqKit: http://www.albahari.com/nutshell/predicatebuilder.aspx
FilterHelper<Country> helper = new FilterHelper<Country>();
helper.AddContains(searchAlpha2, c => c.Alpha2);
helper.AddContains(searchAlpha3, c => c.Alpha3);
helper.AddContains(searchName, c => c.Name);
IQueryable<Country> countries = db.Countries.AsExpandable().Where(helper.Predicate);
...
public class FilterHelper<T>
{
public delegate string GetColumn<T>(T item);
private Expression<Func<T,bool>> predicate;
public FilterHelper()
{
this.predicate = PredicateBuilder.True<T>();
}
public void AddContains(string searchText, GetColumn<T> getColumn)
{
if (!string.IsNullOrWhiteSpace(searchText))
predicate = predicate.And(c => getColumn(c) != null ? getColumn(c).Contains(searchText) : false);
}
public Expression<Func<T,bool>> Predicate
{
get { return this.predicate; }
}
}
c# linq generics
shareimprove this question
edited Jun 22 '14 at 0:35
asked Jun 21 '14 at 21:05
JohnLBevan
20929
closed as off-topic by Jamal♦ Jan 2 at 22:39
This question appears to be off-topic. The users who voted to close gave this specific reason:
"Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Jamal
If this question can be reworded to fit the rules in the help center, please edit the question.
1
To make this work properly, getColumn has to be an expression (e.g. Expression<GetColumn<T>>), not a delegate. With LinqKit, you can then Invoke() that expression in the And() lambda. – svick Jun 22 '14 at 17:32
Thanks @svick; I gave that a go, but because I'm using Linq2SQL (I think) had an issue with Invoke; i.e. LinqKit tries to evaluate it as an SQL expression rather than running this command before passing to SQL. – JohnLBevan Jun 25 '14 at 11:37
That's what AsExpandable() is for: it wraps Linq2SQL (or whatever) IQueryable<T> so that it understands LinqKit's Invoke(). But you already use that, so I don't understand why this isn't working. – svick Jun 25 '14 at 12:01
add a comment
3 Answers
active
oldest
votes
up vote
4
down vote
accepted
I think you're on the right track to reduce repetitive code, but the FilterHelper class hides all the nice PredicateBuilder features when it could instead build on them. The end goal is to create a predicate Expression you can use, so perhaps first start with making the predicate portions themselves reusable:
public static class EntityPredicates
{
public static Expression<Func<T,bool>> PropertyContainsText<T>(string text,
Expression<Func<T,string>> propertySelector)
{
return t => propertySelector.Invoke(t) != null &&
propertySelector.Invoke(t).Contains(text);
}
}
At this point we've got slightly less repetitive code that is pretty expressive about what its trying to do:
var predicate = PredicateBuilder.True<Country>();
if (!string.IsNullOrWhiteSpace(searchAlpha2))
predicate = predicate.And(PropertyContainsText(searchAlpha2, x => x.Alpha2));
if (!string.IsNullOrWhiteSpace(searchAlpha3))
predicate = predicate.And(PropertyContainsText(searchAlpha3, x => x.Alpha3));
if (!string.IsNullOrWhiteSpace(searchAlpha3))
predicate = predicate.And(PropertyContainsText(searchName, x => x.Name));
3
down vote
First bit of cleaning, conditions of the form:
c.Alpha2 != null ? c.Alpha2.Contains(searchAlpha2) : false
are more simply expressed as:
c.Alpha2 != null && c.Alpha2.Contains(searchAlpha2)
If you're not aware, short-circuiting guarantees that the left side will be evaluated before the right, and if it turns out to be false, the right side will not be evaluated as it would be irrelevant to the overall condition. This means that you will never hit a null reference exception.
As for the question, the version in your "Update" section is along the right lines. When trying to access properties, lambdas to pull the property out are always preferable to strings and Reflection when feasible. The code is generally simpler, and you retain all the benefits of working with a strongly typed language.
Debugging why exactly this isn't working is outside the scope of this website, and you'd be better asking that particular chunk on StackOverflow... or, even better, checking if it has already been asked and answered. However, assuming that this can be made to work (and I'd be surprised if it couldn't), it is the right approach.
Beyond that, just a couple of miscellaneous additional points:
There's no need to have separate predicate and Predicate members. Just do:
public Expression<Func<T,bool>> Predicate {get; private set;}
This will achieve what you want much more simply. You will need to replace references to predicate within the class to references to Predicate.
GetColumn doesn't need a generic parameter T as this is already a generic parameter on the class.
shareimprove this answer
answered Jun 22 '14 at 3:48
Ben Aaronson
3,629631
add a comment
up vote
0
down vote
If your main concern is just to remove duplicate code then your code is in a good shape expect few changes like making condition simple as Ben Aaronson as suggested.
I see more opportunities here to make code more useful.
you should have each property condition in different method so in future any change comes for condition only that method would be effected.
for achieving the point 1 you can look for MethodChaining pattern. I have used this pattern and i liked it for such kind of scenario.
public class PerdicateBuilder
{
public Predicate Condition { get; set; }
public Country _country { get; set; }
public PerdicateBuilder(Country country)
{
_country = country;
}
public void SearchCondition(string searchValue, Func propertyName)
{
}
public PerdicateBuilder SearchAlpha2(string searchAlpha2)
{
return this.AddIntoCondition(x => x.Alpha2 != null && x.Alpha2.Contains(searchAlpha2));
}
public PerdicateBuilder SearchAlpha3(string searchAlpha1)
{
return this.AddIntoCondition(x => x.Alpha3 != null && x.Alpha3.Equals(searchAlpha1, StringComparison.OrdinalIgnoreCase));
}
public PerdicateBuilder SearchName(string searchName)
{
return this.AddIntoCondition(x => x.Name != null && x.Name.Contains(searchName));
}
private PerdicateBuilder AddIntoCondition(Predicate predicate)
{
if (Condition == null)
Condition = new Predicate(predicate);
else
Condition += predicate;
return this;
}
public bool EvaluateCondition()
{
return Condition(_country);
}
}
|
If your main concern is just to remove duplicate code then your code
is in a good shape expect few changes like making condition simple as Ben Aaronson
as suggested.
I see more opportunities here to make code more useful.
- you should have each property condition in different method so in
future any change comes for condition only that method would be
effected.
- for achieving the point 1 you can look for MethodChaining pattern. I have used this pattern and i liked it for such kind of scenario.
public class PerdicateBuilder
{
public Predicate Condition { get; set; }
public Country _country { get; set; }
public PerdicateBuilder(Country country)
{
_country = country;
}
public void SearchCondition(string searchValue, Func propertyName)
{
}
public PerdicateBuilder SearchAlpha2(string searchAlpha2)
{
return this.AddIntoCondition(x => x.Alpha2 != null && x.Alpha2.Contains(searchAlpha2));
}
public PerdicateBuilder SearchAlpha3(string searchAlpha1)
{
return this.AddIntoCondition(x => x.Alpha3 != null && x.Alpha3.Equals(searchAlpha1, StringComparison.OrdinalIgnoreCase));
}
public PerdicateBuilder SearchName(string searchName)
{
return this.AddIntoCondition(x => x.Name != null && x.Name.Contains(searchName));
}
private PerdicateBuilder AddIntoCondition(Predicate predicate)
{
if (Condition == null)
Condition = new Predicate(predicate);
else
Condition += predicate;
return this;
}
public bool EvaluateCondition()
{
return Condition(_country);
}
}
Calling would be like
var perdicateBuilder = new PerdicateBuilder(country);
perdicateBuilder.SearchAlpha2(searchAlpha2)
.SearchAlpha3(searchAlpha3)
.SearchName(searchName);
IQueryable<Country> countries = db.Countries.AsExpandable().Where(x => perdicateBuilder.EvaluateCondition());
|
As you can see in the above code i have used each condition as different method as you have already created,
let's say in future you need to change the condition of
Alpha3 the you just need to change the method implementation of
SearchAlpha3 as -
public PerdicateBuilder SearchAlpha3(string searchAlpha1)
{
return this.AddIntoCondition(x => x.Alpha3 != null && x.Alpha3.Equals(searchAlpha1, StringComparison.OrdinalIgnoreCase));
}
