0

I'm getting some Entities from EF, which I iterate and creating Anonymous type objects such as:

var payments = ctx.Payments.ToList();
var data = ctx.Activities.OrderBy(p => p.ID).ToList().Select(p => new
{
    ID = p.ID,
    Date = p.Date?.ToString("dd/MM/yyyy"),
    PaymentMethod = p.PaymentMethods != null ? p.PaymentMethods.Description : "",
    ActivityStatusID = payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() == 0 ? 1 : .. // I need some other check
}).ToList();

Now, I'd like to check the payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() several times before set a value.

Such as:

  1. if .Count() == 0, value 1
  2. if .Count() == 1, value 8
  3. if .Count() > 1, value 10
  4. else 87

and so on. I won't do somethings like this:

ActivityStatusID = payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() == 0 ? 1 : payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() == 1 ? 8 : payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() > 1 ? 10 : 87

Is there any way to do payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() once for each p, and than evalutate the conditions?

markzzz
  • 47,390
  • 120
  • 299
  • 507
  • You could use `let` if you were using query syntax instead of method syntax. If you want to stick to method syntax, you have to introduce another anonymous type that has `payments` and a property which value is `payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count() ` – vc 74 Jul 18 '18 at 09:52

2 Answers2

1

I would change the query from a lambda expression to an ordinary query. Then use the let syntax to set a variable for each iteration. Something like this:

var payments = ctx.Payments.ToList();
var data = (
    from p in ctx.Activities.ToList()
    orderby p.ID
    let paymentCount = payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count()
    select new
    {
        ID = p.ID,
        Date = p.Date?.ToString("dd/MM/yyyy"),
        PaymentMethod = p.PaymentMethods != null ? p.PaymentMethods.Description : "",
        ActivityStatusID = paymentCount == 0 ? 1 : .. // I need some other check
    }
).ToList();

And btw you can do this part diffrently as well. This:

payments.Where(q => q.ActivityID == p.ID && !q.Paid).Count()

You can write like this:

payments.Count(q => q.ActivityID == p.ID && !q.Paid)
Arion
  • 31,011
  • 10
  • 70
  • 88
  • Tried your suggestions. Some troubles. The first: can't do `Date = p.Date?.ToString("dd/MM/yyyy")` (an expression tree lambda may not contain a null propagating operator). So I've changed it to `Date = p.Date.HasValue ? p.Date.Value.ToString("dd/MM/yyyy") : null,` , but now "LINQ to Entities does not recognize the method 'System.String ToString(System.String)' method, and this method cannot be translated into a store expression." I think you forgot `ctx.Activities.ToList()`? – markzzz Jul 18 '18 at 12:11
  • P.s. why I can't use `let` with Lambda? – markzzz Jul 18 '18 at 12:34
  • @markzzz : True I forgot the .ToList() in the end. I have updated the answer – Arion Jul 18 '18 at 12:52
  • @markzzz : Because you do not have a "query body" to set the let in – Arion Jul 18 '18 at 12:53
1

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.

Harald Coppoolse
  • 28,834
  • 7
  • 67
  • 116