C# Code Optimization GroupBy

Question

I have a couple of problems with my below code.

var groupedItems = inputFiles.GroupBy(q => q.Name.ToLower().Split('_').ElementAt(2));
        string currentNo = ////value retreived from someMethod;
        if (string.IsNullOrEmpty(currentNo))
        {
            if (groupedItems.Count() > 1)
            {
                foreach (var group in groupedItems)
                {
                    foreach (var groupedItem in group)
                    {
                        ErrorFile(groupedItem);
                    }
                }
            }
            else if (groupedItems.Count() == 1)
            {
                ProcessFile();
            }
        }
        else
        {
            foreach (var group in groupedItems.Where(x => x.Key != currentNo))
            {
                foreach (var groupedItem in group)
                {
                    ErrorFile(groupedItem);
                }
            }
        }

There is repetitive code with nested foreach loops. I am looking at possibility of optimization
How do I handle when Split and ElementAt(2) return errors. I still need to call ErrorFile() method even if I am unable to Split by _.

InBetween · Accepted Answer · 2017-01-17T11:15:10.530

Hard to understand what your code is really doing without more context, but this should be about right:

static void SomeMethod(IEnumerable<File> inputFiles)
{
    var groupedItems = inputFiles.GroupBy
        (f => splitAndFindElement(f.Name, '_', 2, string.Empty));

    string currentNo = //whatever;

    if (string.IsNullOrEmpty(currentNo))
    {
        if (groupedItems.Count() > 1)
        {
            foreach (var item in groupedItems.SelectMany(g => g))
            {
                ErrorFile(item);
            }
        }
        else if (groupedItems.Count() == 1)
        {
            ProcessFile();
        }
    }
    else
    {
        foreach (var item in groupedItems.Where(g => g.Key != currentNo).SelectMany(g => g))
        {
            ErrorFile(item);
        }
    }
}

And the helper method:

static string splitAndFindElement(string input, char splitter, int index, string resultOnFail)
{
    var succesful = false;
    string[] words = null;

    if (input != null)
    {
        words = input.Split(splitter);
        succesful = words.Length > index;
    }

    return succesful ? words[index] : resultOnFail;
}

The trick here is to group files with key "" if they can't be split. This will ensure that they will be processed with ErrorFile either becuase there is more than one grouping or because the key does not equal currentNo. I'm assuming here that Name can't end in "_".

Also, SelectMany is used to flatten enumerables of enumerables and avoid nested loops.

score 0 · Answer 2 · answered Jan 17 '17 at 14:17

We can optimize repeated loops as following, and to handle error use try catch

var groupedItems;
try
{
    groupedItems= inputFiles.GroupBy(q => q.Name.ToLower().Split('_').ElementAt(2));

    string currentNo = ////value retreived from someMethod;
    if (string.IsNullOrEmpty(currentNo) && groupedItems.Count() == 1)
    {
        ProcessFile();
    }
    else
    {
        foreach (var group in groupedItems.Where(x => string.IsNullOrEmpty(currentNo) || x.Key != currentNo))
        {
            foreach (var groupedItem in group)
            {
                ErrorFile(groupedItem);
            }
        }
    }
}
catch
{
    ErrorFile(groupedItem);
}

C# Code Optimization GroupBy

2 Answers2