9
\$\begingroup\$

I want to filter search results by multiple filters at once. Is it possible to reduce the number of if statements? My code:

    public IEnumerable<Article> Search(ArticleFiltersModel filters, ArticleSortOptions? options)
    {
        var result = Mapper.Map<IQueryable<Article>>(_unitOfWork.Articles.AsQueryable());
        if (filters != null)
            Filter(ref result, filters);
        else
            result = result.Where(article => article.IsAvailable == true);
        if (options.HasValue)
            Sort(ref result, options.Value);
        return result;
    }

    private void Filter(ref IQueryable<Article> articles, ArticleFiltersModel filters)
    {
        if (filters.IsAvailable.HasValue)
            articles = articles.Where(article => article.IsAvailable == filters.IsAvailable);
        if (!string.IsNullOrEmpty(filters.Name))
            articles = articles.Where(article => article.Title.Contains(filters.Name));
        if (filters.AreaFrom.HasValue)
            articles = articles.Where(article => article.House.Area >= filters.AreaFrom);
        if (filters.AreaTo.HasValue)
            articles = articles.Where(article => article.House.Area <= filters.AreaTo);
        if (filters.PriceFrom.HasValue)
            articles = articles.Where(article => article.House.Area >= filters.PriceFrom);
        if (filters.PriceTo.HasValue)
            articles = articles.Where(article => article.House.Area <= filters.PriceTo);
        if (filters.Type.HasValue)
            articles = articles.Where(article => article.House.Type == filters.Type);
        if (filters.RoomsCount.HasValue)
            articles = articles.Where(article => article.House.RoomsCount == filters.RoomsCount);
        if (filters.Floors.HasValue)
            articles = articles.Where(article => article.House.FloorsCount == filters.Floors);
        if (filters.Floor.HasValue)
            articles = articles.Where(article => article.House.Floor == filters.Floor);
        if (filters.HasParking.HasValue)
            articles = articles.Where(article => article.House.HasParking == filters.HasParking);
        if (filters.WithHomeAppliances.HasValue)
            articles = articles.Where(article => article.House.WithHomeAppliances == filters.WithHomeAppliances);
    }

My filter model:

public class ArticleFiltersModel
{
    public string Name { get; set; }
    public decimal? AreaFrom { get; set; }
    public decimal? AreaTo { get; set; }
    public decimal? PriceFrom { get; set; }
    public decimal? PriceTo { get; set; }
    public HouseType? Type { get; set; }
    public int? RoomsCount { get; set; }
    public bool? IsFurnished { get; set; }
    public int? Floors { get; set; }
    public int? Floor { get; set; }
    public bool? HasParking { get; set; }
    public bool? WithHomeAppliances { get; set; }
    public bool? IsAvailable { get; set; }
}

UPDATE

Article.cs:

public class Article
{
    public int Id { get; set; }
    public string Title { get; set; }
    public int HouseId { get; set; }
    public House House { get; set; }
    public string Description { get; set; }
    public DateTime Added { get; set; }
    public bool IsAvailable { get; set; }

    public ICollection<Realtor> Realtors { get; set; }
    public ICollection<Image> Images { get; set; }
}

House.cs:

public enum HouseType { House, Apartment, Condo, Cooperative, Land, Office,
                        Restaurant, Bar, Storage, Building, Other }
public class House
{
    public int Id { get; set; }
    public decimal Area { get; set; }
    public decimal Price { get; set; }
    public int RoomsCount { get; set; }
    public bool IsFurnished { get; set; }
    public int FloorsCount { get; set; }
    public int Floor { get; set; }
    public bool HasParking { get; set; }
    public bool WithHomeAppliances { get; set; }
    public int AddressId { get; set; }
    public Address Address { get; set; }
    public HouseType Type { get; set; }
    public int PhotoId { get; set; }
    public Image Photo { get; set; }
}
\$\endgroup\$
11
\$\begingroup\$

I read it as one long AND operation where the result is the articles that satisfy all the valid predicates. You could therefore build an enumerable of valid predicates in an extension method:

  public static class ArticleFilterExtensions
  {
    public static IEnumerable<Predicate<Article>> GetValidPredicates(this ArticleFiltersModel filter)
    {
      if (filter.IsAvailable.HasValue)
        yield return a => a.IsAvailable == filter.IsAvailable;
      if (!string.IsNullOrWhiteSpace(filter.Name))
        yield return a => a.Title.Contains(filter.Name);
      if (filter.AreaFrom.HasValue)
        yield return a => a.House.Area >= filter.AreaFrom;
      // etc.

      if (filter.WithHomeAppliances.HasValue)
        yield return a => a.House.WithHomeAppliances == filter.WithHomeAppliances;
    }
  }

And your filter method could then be reduced to:

private void Filter(ref IQueryable<Article> articles, ArticleFiltersModel filters)
{
  var predicates = filters.GetValidPredicates().ToList();
  articles = articles.Where(a => predicates.All(p => p(a)));
}

You could then easily create an OR filter as

private void OrFilter(ref IQueryable<Article> articles, ArticleFiltersModel filters)
{
  var predicates = filters.GetValidPredicates().ToList();
  articles = articles.Where(a => predicates.Any(p => p(a)));
}

`

\$\endgroup\$
  • 1
    \$\begingroup\$ +1 for using a single call to Where and reusing predicates for or, and \$\endgroup\$ – dfhwze Jul 29 at 17:50
  • 1
    \$\begingroup\$ @dfhwze: Maybe GetValidPredicates() should return an array or a readonly list instead of yield-ing? \$\endgroup\$ – Henrik Hansen Jul 29 at 17:56
  • \$\begingroup\$ I think we are micro-optimising in such case. But a comparative test with a huge amount of predicates could yield us more info. \$\endgroup\$ – dfhwze Jul 29 at 17:59
  • \$\begingroup\$ @dfhwze this would be a pretty academic test because which sane person is using a huge amount of predicates in the real-world? :-P \$\endgroup\$ – t3chb0t Jul 29 at 18:12
  • 1
    \$\begingroup\$ @dfhwze I guess I should answer this question with luckily not insteady of unfortunately haha \$\endgroup\$ – t3chb0t Jul 29 at 18:15
5
\$\begingroup\$

I would further improve Henrik Hansen's code by throwing away the ifs and integrating the preconditions inside in the queries so they would become:

yield articles.Where(article => !filters.IsAvailable.HasValue || article.IsAvailable == filters.IsAvailable);

This would not only make it more readable but would show us that now we can actually generate these expressions (probably with Jesse C. Slicer's code) which would save us a lot of typing.

\$\endgroup\$
  • \$\begingroup\$ Henrik returns predicates, later used to get articles, where you return articles directly. Did you mean to inline the if in the predicates or directly in the result set? \$\endgroup\$ – dfhwze Jul 29 at 18:48
  • 1
    \$\begingroup\$ In this way you query every filter for every article where mine filter the filters once and then only use the valid filters on each article. \$\endgroup\$ – Henrik Hansen Jul 29 at 18:56
  • \$\begingroup\$ @HenrikHansen of course but the sql-server doesn't care, it can optmize it away... I'm pretty sure of that. \$\endgroup\$ – t3chb0t Jul 29 at 19:00
  • 1
    \$\begingroup\$ @dfhwze I cannot find any signifficant difference in the two execution plans, even with the help of the execution-plan comparer. I wonder whether someone already has asked this question on stack-overflow... \$\endgroup\$ – t3chb0t Jul 29 at 19:30
  • 1
    \$\begingroup\$ @t3chb0t - there are a lot of things at play here. I can tell you that I have seen an 80% reduction in cost in EF for a complex query by "flattening" the predicates into a single Where rather than using multiple Where calls. Projection (Select) and navigation property use can have big impacts on generated SQL - hard to test without a specific case! \$\endgroup\$ – RobH Jul 31 at 9:36
3
\$\begingroup\$

Given an extension method:

public static IQueryable<T> NullWhere<T>(this IQueryable<T> source, Expression<Func<T, bool?>> expression, bool? compare) => compare.HasValue
    ? source.Where(Expression.Lambda<Func<T, bool>>(Expression.Equal(expression.Body, Expression.Constant((bool)compare)), expression.Parameters))
    : source;

You can then use:

articles = articles.NullWhere(article => article.IsAvailable, filters.IsAvailable);

Note this works for equality for nullable bools - you'll have to create other extension methods for nullable decimals with greater-than-or-equal-to, null/empty strings with string.Contains(), etc. But the principle will be the same behind each.

\$\endgroup\$
  • 2
    \$\begingroup\$ I'm contemplating whether Expresssion is really required here. Why not just use the Func instead? \$\endgroup\$ – dfhwze Jul 29 at 18:26
  • 2
    \$\begingroup\$ @dfhwze expressions don't have to be required. They are cool by themselfes wherever used ;-] \$\endgroup\$ – t3chb0t Jul 29 at 18:31
  • 1
    \$\begingroup\$ @dfhwze oh, this could end up recursive... who would be generating who? A T4 template for generating expressions to generate T4 templates to generate code generating expressions generating T4 templates... a very deep rabitt hole. \$\endgroup\$ – t3chb0t Jul 29 at 18:36
2
\$\begingroup\$

How about a generic extension method that takes a boolean and an expression, whereas the expression is only evaluated in case the boolean is true:

public static IQueryable<T> When<T>(
    this IQueryable<T> source, bool trigger, Expression<Func<T, bool>> expression)
{
    if (trigger)
    {
        return source.Where(expression);
    }

    return source;
}

It allows you to chain the calls similar to what @t3chb0t suggested, but it will not even chain unnecessary filters:

articles = articles.When(filters.IsAvailable.HasValue, article => article.IsAvailable == filters.IsAvailable)
    .When(!string.IsNullOrEmpty(filters.Name), article => article.Title.Contains(filters.Name));

Similarly it would be possible to write an even more generic method that doesn't take Expression<Func<T, bool>> as third parameter but an Expression<Func<IQueryable<T>, IQueryable<T>>>, which would allow you to chain arbitrary methods (with a trigger).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.