I have written a piece of code which builds an EF Core Where() predicate at runtime. I have never done this before, but for the requirement I had to do it.

The requirement is simply to get data from the db within 7 days of CreatedOn or UpdatedOn.

The previous version before the refactor was to call the database 7 time in a loop which had performance problems.

The current implementation works as expected and returns the expected results.

This is the current code:

private async Task SevenDaysCashOutFloor(DateTimeOffset today, IQueryable<BillPaymentVoucher> pastBillPayments, IQueryable<JournalVoucherPaymentVoucher> pastJournalVoucherPayments, CancellationToken token)
{
        Expression<Func<BillPaymentVoucher, bool>> predicate = null!;
        Expression<Func<BillPaymentVoucher, bool>> aggregatedPredicate = null!;
        BinaryExpression binaryExpression = null!;

        var param = Expression.Parameter(typeof(BillPaymentVoucher));

        today = DateTimeOffset.UtcNow;

        for (int days = 0; days < 7; days  )
        {
            var date = today.AddDays(-days);

            predicate = (entity) => (
                                        (entity.UpdatedOn.HasValue && entity.UpdatedOn.Value.Date == date.Date) ||
                                        (entity.UpdatedOn.HasValue == false && entity.CreatedOn.Date == date.Date)
                                    );

            binaryExpression = Expression.OrElse(ExpressionReplacer.GetBody(aggregatedPredicate ?? predicate, param),
                                                 ExpressionReplacer.GetBody(predicate, param));

            aggregatedPredicate = Expression.Lambda<Func<BillPaymentVoucher, bool>>(binaryExpression, param);
        }

        var finalPredicate = Expression.Lambda<Func<BillPaymentVoucher, bool>>(binaryExpression, param);

        pastBillPayments = pastBillPayments.Where(finalPredicate);          
}

I am building the predicate at runtime which is needed by the Where() in a for loop.

It all works as expected, but I want to know:

1. if this is the proper way of writing it?

2. should I be using the Expression Tree which is complex?

3. is there a simple way to achieve this?

4. can this code be refactored?

I have never worked with expression trees before.

CodePudding user response：

Where is always constructed at runtime. There's no need to explicitly check for nulls either. The expressions will be translated to SQL which has explicit support for NULL values.

This method searches for rows whose UpdatedOn or CreatedOn fall in a range of days. You can probably replace it by creating a list of those days and using Contains :

var daysInWeek=Enumerable.Range(0,7)
                         .Select(i=>DateTime.Today.AddDays(-i))
                         .ToList();

pastBillPayments = pastBillPayments.Where(p=>daysInWeek.Contains(p.UpdatedOn.Date) 
                          || daysInWeek.Contains(p.CreatedOn.Date) );

This will generate :

WHERE ... cast(UpdatedOn as date) IN (@d1,@d2,...,@d7) OR 
          cast(CreatedOn as date) IN (@d1,@d2,...,@d7)

That's not optimal though. While some databases (eg SQL Server) can convert cast(UpdatedOn as date) = ... into a range search that can use indexes, they can't use the index statistics.

An even better query would be to explicitly search in a date range :

var dateTo=DateTime.Today.AddDays(1);
var dateFrom=DateTime.Today.AddDays(-6);

pastBillPayments = pastBillPayments
    .Where(p=> 
        (p.UpdatedOn >= dateFrom && p.UpdateOn  < dateTo) ||
        (p.CreatedOn >= dateFrom && p.CreatedOn < dateTo));

This eliminates any casting, allowing the server to use both indexes and statistics