People suggested to convert your query into query syntax, which would enable your to use a let
statement to create a variable in which you could save count.
If you investigate what let
does, it adds one column value to your result. Every row in your result has the same let value in this column. See this answer on stackoverflow
Keep your query in Method Syntax
If you want to keep your query in method syntax, simply add a Count to your anonymous type:
var result = ctx.Activities
.OrderBy(p => p.ID)
.Select(p => new
{
Id = p.Id,
Date = p.Date?.ToString("dd/MM/yyyy"),
PaymentMethod = p.PaymentMethods != null ? p.PaymentMethods.Description : "",
PaidCount = payments
.Where(q => q.ActivityID == p.ID && !q.Paid)
.Count();
})
.Select(p => new
{
Id = p.Id,
Date = p.Date,
ActivityStatusId =
{
// count == 0 => 1
// count == 1 => 8
// count > 1 => 10
// count < 0 => 87
if (p.PaidCount < 0) return 87;
switch (p.PaidCount)
{
case 0:
return 0;
case 1:
return 8;
default:
return 10;
}
},
});
Note, the switch is only possible because you brought your complete ctx.Activities to local memory using the ToList.
Improved efficiency
You do a ToList
before your select. This means that your complete ctx.payments are materialized in local memory into a List
, before you start selecting and counting items in your sequence.
If ctx.Payments is from an external source, like a database, or a file, then ctx.Payments is an IQueryable
instead of an IEnumerable
. Fetching your complete Payments to local memory is not an efficient approach.
Advise: Whenever you have an IQueryable, try to keep it IQueryable as long as possible. Your source data provider can process your queries much more efficiently than your local processor. Only materialize it to local memory if your source data provider can't process it anymore, for instance because you need to call local procedures, or because there is nothing to process anymore.
Furthermore, don't move values to local memory that you don't plan to use. Only Select the properties that you actually will use in your local memory.
One improvement would be:
var result = ctx.Payments.Select(payment => new
{ // select only the properties you plan to use locally:
Id = payment.Id,
Date = payment.Date,
PaymentMethod = payment.PaymentMethods?.Description,
PaidCount = ctx.Payments
.Where(q => q.ActivityID == p.ID && !q.Paid)
.Count(),
})
.OrderBy(fetchedPaymentData => fetchedPaymentData.Id)
// from here you need to move it to local memory
// Use AsEnumerable instead of ToList
.AsEnumerable()
.Select(fetchedPaymentData => new
{
Id = fetchedPaymentData.Id,
PaymentMethod = fetchedPaymentData.PaymentMethod ?? String.Empty,
ActivityStatusId = {...}
});
AsEnumerable is more efficient than ToList, especially if you don't need all items at once. For instance if you would end with FirstOrDefault, or only Take(5), then it would be a waste to move all items to local memory.
Finally: with some trying you can get rid of the switch statement, thus allowing your DBMS to calculate the ActivityStatusId. But as the selecting of your source data and the transport of the selected data to local memory is the slower part of your complete query, I doubt whether this would lead to shorter execution time. The switch surely makes your requirement better readable, especially if your numbers 1 / 8 / 87 are put into enums.