4

The code I'm using is here:

            var phraseCounts = new PhraseCounts()
            {
                All = phrases.Count(),
                JLPT1 = phrases.Count(x => x.Jlpt == 1),
                JLPT2 = phrases.Count(x => x.Jlpt == 2),
                JLPT3 = phrases.Count(x => x.Jlpt == 3),
                JLPT4 = phrases.Count(x => x.Jlpt == 4),
                JLPT5 = phrases.Count(x => x.Jlpt == 5),
                Hidden = phrases.Count(x => x.Hidden == true),
                F1 = phrases.Count(x => x.F1 == true),
                F2 = phrases.Count(x => x.F2 == true),
                F3 = phrases.Count(x => x.F3 == true),
                F4 = phrases.Count(x => x.F4 == true),
                F5 = phrases.Count(x => x.F5 == true),
            };

The code works but I wonder if there is a different way to do this.

Alan2
  • 23,493
  • 79
  • 256
  • 450
  • Not in a single statement, but less... Take a look [here](https://stackoverflow.com/questions/7285714/linq-with-groupby-and-count) – Peter Schneider Nov 28 '19 at 23:18
  • I'm not sure but the difficulty I might have is getting all the counts to go into the different properties of the phraseCounts object. – Alan2 Nov 28 '19 at 23:21
  • You could itereate once and inside your for or foreach increment each field based on the condition... So having multiple IF statements inside your loop.... This way at least you will iterate only once. – Jonathan Alfaro Nov 29 '19 at 01:48
  • What's wrong with this? If it works, then just be satisfied with it. Compacting and reducing code is sometimes overrated. Readability and simplicity are more important than clever code compaction. – robbpriestley Nov 29 '19 at 02:36
  • 1
    _if it ain't broke, don't fix it_ – Thangadurai Nov 29 '19 at 04:49

1 Answers1

1

There are many different ways of doing it, whether or not there is any actual benefit for one particular form depends a lot on what your data is and where it's coming from.

If enumerating your phrases collection is costly, let's say it reads a big file, then you may want to read it all to memory and then execute your counts. For this just adding a .ToList() call prior to counting would be fine.

 var inMemoryPhrases = phrases.ToList(); 
 var phraseCounts = new PhraseCounts()
            {
                All = inMemoryPhrases.Count(),
                JLPT1 = inMemoryPhrases.Count(x => x.Jlpt == 1),
                JLPT2 = inMemoryPhrases.Count(x => x.Jlpt == 2),
                JLPT3 = inMemoryPhrases.Count(x => x.Jlpt == 3),
                JLPT4 = inMemoryPhrases.Count(x => x.Jlpt == 4),
                JLPT5 = inMemoryPhrases.Count(x => x.Jlpt == 5),
                Hidden = inMemoryPhrases.Count(x => x.Hidden == true),
                F1 = inMemoryPhrases.Count(x => x.F1 == true),
                F2 = inMemoryPhrases.Count(x => x.F2 == true),
                F3 = inMemoryPhrases.Count(x => x.F3 == true),
                F4 = inMemoryPhrases.Count(x => x.F4 == true),
                F5 = inMemoryPhrases.Count(x => x.F5 == true),
            };

I know, I know, the actual form didn't change, we are still doing multiple count calls on an underlying collection.

Now suppose the underlying collection enumeration is costly and the dataset is just too big to fit in memory. In this case reading it all to memory won't work and we don't want to enumerate the collection multiple times. In this case you can compute the counts incrementally. It may not look as pretty as the declarative linq option but in this scenario it is probably a better choice.

 var phraseCounts = new PhraseCounts() {
                All = 0,
                JLPT1 = 0,
                JLPT2 = 0,
                JLPT3 = 0,
                JLPT4 = 0,
                JLPT5 = 0,
                Hidden = 0,
                F1 = 0,
                F2 = 0,
                F3 = 0,
                F4 = 0,
                F5 = 0,
            };

 foreach (var x in phrases)
 {
    phraseCounts.All++;
    switch(x.Jlpt) 
    {
        case 1: phraseCounts.JLPT1 ++; break;
        case 2: phraseCounts.JLPT2 ++; break;
        case 3: phraseCounts.JLPT3 ++; break;
        case 4: phraseCounts.JLPT4 ++; break;
        case 5: phraseCounts.JLPT5 ++; break;
    }

    if (x.Hidden) phraseCounts.Hidden ++;
    if (x.F1) phraseCounts.F1++;
    if (x.F2) phraseCounts.F2++;
    if (x.F3) phraseCounts.F3++;
    if (x.F4) phraseCounts.F4++;
    if (x.F5) phraseCounts.F5++;
};

The same can be achieved using an aggregator lambda:

var phraseCounts = phrases.Aggregate(
    new PhraseCounts() {
                All = 0,
                JLPT1 = 0,
                JLPT2 = 0,
                JLPT3 = 0,
                JLPT4 = 0,
                JLPT5 = 0,
                Hidden = 0,
                F1 = 0,
                F2 = 0,
                F3 = 0,
                F4 = 0,
                F5 = 0,
            }, 
    (total, x) => {
        total.All++;
        switch(x.Jlpt) 
        {
            case 1: total.JLPT1 ++; break;
            case 2: total.JLPT2 ++; break;
            case 3: total.JLPT3 ++; break;
            case 4: total.JLPT4 ++; break;
            case 5: total.JLPT5 ++; break;
        }

        if (x.Hidden) total.Hidden ++;
        if (x.F1) total.F1++;
        if (x.F2) total.F2++;
        if (x.F3) total.F3++;
        if (x.F4) total.F4++;
        if (x.F5) total.F5++;
        return total;
    });

Now let's say phrases is actually an IQueryable with the data coming from a database using EntityFramework, NHibernate, XPO or something else that parse expression trees and generates sql commands. In this case you would want to avoid calling your database multiple times and also bringing individual rows as you just need the counts. In this case you can resolve everything using one linq expression instead many, which should be translatable to a single sql statement.

var phraseCounts = phrases.GroupBy(x => 1).Select(g => new PhraseCounts()
{
    All = g.Count(),
    JLPT1 = g.Count(x => x.Jlpt == 1),
    JLPT2 = g.Count(x => x.Jlpt == 2),
    JLPT3 = g.Count(x => x.Jlpt == 3),
    JLPT4 = g.Count(x => x.Jlpt == 4),
    JLPT5 = g.Count(x => x.Jlpt == 5),
    Hidden = g.Count(x => x.Hidden == true),
    F1 = g.Count(x => x.F1 == true),
    F2 = g.Count(x => x.F2 == true),
    F3 = g.Count(x => x.F3 == true),
    F4 = g.Count(x => x.F4 == true),
    F5 = g.Count(x => x.F5 == true),
});

I only considered some context variations for creating these versions but this is a non-exhaustive list, I'm sure we can express the same in far more different ways.

Victor Ortuondo
  • 406
  • 3
  • 6