patterncsharpMinor
Calculating user efficiency on a time period with Linq
Viewed 0 times
linqperiodwithusertimecalculatingefficiency
Problem
I have this expression:
This expression calculates the user's efficiency by calculating the percentage of successful orders from a total of the last 30 days.
How can I make it more clear?
Update:
There is a second approach of writing the expression, but it generates more expensive SQL query.
public static readonly Expression> EfficiencyPercentExpr = e =>
(e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30)).Count(x => x.Status != Order.OrderStatus.Deleted) > 0)
?
(
e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30)).Count(x => x.Status != Order.OrderStatus.Deleted)
/
e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30)).Count()
)
:
0
;This expression calculates the user's efficiency by calculating the percentage of successful orders from a total of the last 30 days.
How can I make it more clear?
Update:
There is a second approach of writing the expression, but it generates more expensive SQL query.
private static readonly Expression> efficiencyPercentExpr = e =>
e.ExecutorOrders.Where(x => !x.IsNotConsidered && x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30))
.GroupBy(x => 1, (k, g) => new { Count = g.Count(), CountUndeleted = g.Count(x => x.Status != Order.OrderStatus.Deleted) })
.Select(x => x.CountUndeleted > 0 ? (decimal)x.CountUndeleted / (decimal)x.Count : 0).FirstOrDefault();Solution
- Don't one line things that shouldn't be one lined. Just because you can do something, doesn't mean that you should.
- Extract the duplicated query into a method of it's own that returns an
IQueryable
-
Don't query the database twice for the same count, execute the query and store it in a variable.
private IQueryable ExecutorOrdersInLast30Days(ApplicationUser e)
{
return e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30));
}
private decimal EfficiencyPercent(ApplicationUser e)
{
var nonDeletedOrders = ExecutorOrdersInLast30Days(e).Count(x => x.Status != Order.OrderStatus.Deleted);
if ( nonDeletedOrders > 0)
{
return nonDeletedOrders / ExecutorOrdersInLast30Days(e).Count();
}
else
{
return 0;
}
}Code Snippets
private IQueryable<foo> ExecutorOrdersInLast30Days(ApplicationUser e)
{
return e.ExecutorOrders.Where(x => x.Added >= DbFunctions.AddDays(SqlFunctions.GetUtcDate(), -30));
}
private decimal EfficiencyPercent(ApplicationUser e)
{
var nonDeletedOrders = ExecutorOrdersInLast30Days(e).Count(x => x.Status != Order.OrderStatus.Deleted);
if ( nonDeletedOrders > 0)
{
return nonDeletedOrders / ExecutorOrdersInLast30Days(e).Count();
}
else
{
return 0;
}
}Context
StackExchange Code Review Q#151088, answer score: 7
Revisions (0)
No revisions yet.