Linq Ternary Code Better Solution

Question

I have this code a developer wrote. I think its horrible and shouldnt be necessary

value = s.Businesses.SelectMany(
    x => x.Payments.Where(
        w => w.total != 0 && 
        !w.jobId.HasValue && 
        w.createdAt >= Utility.monthS 
        && w.createdAt <= Utility.monthE)
    ).Any() ? 
        s.Businesses.SelectMany(
            x => x.Payments.Where(
                w => w.total != 0 && 
                !w.jobId.HasValue && 
                w.createdAt >= Utility.monthS 
                && w.createdAt <= Utility.monthE)
            ).Sum(su => su.quantity) 
        : 0;

The reason it does the .Any before the Sum is that records with no values end up getting null values and causing errors.

Is there a better best practice way of writing this.

`Enumerable.Sum` when called on an empty collection will return 0. — juharr, Sep 09 '15 at 13:07
I would recommend at the very least to assign the `SelectMany` result to a variable so you only have to go over it once. Then use .Any and .Sum on the variable. This will result in a much cleaner look. — David, Sep 09 '15 at 13:17

juharr · Accepted Answer · 2015-09-14T11:52:00.423

7

If this wasn't for Entity Framework then Sum would just return 0 for empty collections and you wouldn't need to do the Any

value = s.Businesses.SelectMany(
    x => x.Payments.Where(
        w => w.total != 0 && 
             !w.jobId.HasValue && 
             w.createdAt >= Utility.monthS && 
             w.createdAt <= Utility.monthE))
    .Sum(su => su.quantity);

However since this is for Entity Framework you have the issue of it being turned into SQL and SUM in T-SQL will return null for an empty set. So you have to replace that last line with one of the following to make it work

.Sum(su => (int?)su.quanity) ?? 0;

or

.Select(su => su.quanity).DefaultIfEmpty().Sum();

The first will tell C# to expect a possible null and to use 0 if it is null. The second will replace an empty result with a set with a single default value, in this case 0.

You do not want to do Any and then a Sum because that will result in two calls to the DB.

edited Sep 14 '15 at 11:52

answered Sep 09 '15 at 13:12

juharr

31,741
4
58
93

This works fine when I run it locally. And does in fact return 0. However when deployed gets an error as though its returning nulls. This is why the developer implemented this solution. See here for more info on the error http://stackoverflow.com/questions/6864311/the-cast-to-value-type-int32-failed-because-the-materialized-value-is-null – Dale Fraser Sep 09 '15 at 23:21
@DaleFraser The answer there was to cast to `int?` and use `??` or `DefaultIfEmpty(0).Sum()`, not to use `Any`. – juharr Sep 10 '15 at 12:00
However this @juharr answer is wrong. "But since Sum will just return 0 for empty collections you don't need to do the Any" but that is not the case. – Dale Fraser Sep 13 '15 at 22:59
@DaleFraser Yes, it's not entirely correct for EF and I've updated to express that, but I still maintain that you don't need to do the `Any`. – juharr Sep 14 '15 at 11:52

Alexander · Answer 2 · 2015-09-09T13:30:43.800

1

This is how it will look like in query syntax:

var payments = 
  from x in s.Businesses
  from w in x.Payments
  where 
  w.total != 0 && 
  !w.jobId.HasValue && 
  w.createdAt >= Utility.monthS && 
  w.createdAt <= Utility.monthE
  select w;

value = 
payments.Any() ? payments.Sum(p => p.Quantity) : 0;

As other posters commented - Any() may not even be neccessary, so last line could be value = payments.Sum(p => p.Quantity)

edited Sep 09 '15 at 13:30

answered Sep 09 '15 at 13:20

Alexander

4,153
1
24
37

Why not just `value = (from ... select w.Quantity).Sum();`? Also this is called query syntax, not pure linq. – juharr Sep 09 '15 at 13:24
Thanks, @juharr. I didn't know that, always called it pure :). Modified my answer. – Alexander Sep 09 '15 at 13:31

Linq Ternary Code Better Solution

2 Answers2