CODEWITHSUNDEEP
c#linqlistforeach
Midhun
Midhun

Reputation: 393

ForEach for list of list in Linq

I am trying to do some operations with list of list.

I have a list with some properties. I have splitted the list into sublists based on GroupId Property.

I am assigning each listitems in the list to another class. but here i had to use 2 ForEachs. is there any way to do it in one so that i can increase the performance when it comes to a large input of more than 10000 list items in the input list. below is the code I tried

 class Comp
 {
    public int CompId { get; set; }
    public string CompName { get; set; }
    public int GroupId { get; set; }
 }

 class EmpComp
 {
        public int EmpCompId { get; set; }
        public string EmpCompName { get; set; }
        public int EmpId { get; set; }
        public int GroupId { get; set; }
 }

    #region Input

    List<Comp> compList = new List<Comp>();
    compList.Add(new Comp { CompId = 1, CompName = "One", GroupId = 1 });
    compList.Add(new Comp { CompId = 2, CompName = "Two", GroupId = 1 });
    compList.Add(new Comp { CompId = 3, CompName = "One", GroupId = 2 });
    compList.Add(new Comp { CompId = 4, CompName = "Three", GroupId = 1 });
    compList.Add(new Comp { CompId = 5, CompName = "One", GroupId = 4 });
    compList.Add(new Comp { CompId = 6, CompName = "Two", GroupId = 4 }); 

    #endregion

    var groupedCompList = compList.GroupBy(u => u.GroupId ).Select(grp => grp.ToList()).ToList();
    List<EmpComp> empCompList = new List<EmpComp>();
    int empId = 0;//Just for reference
    groupedCompList.ForEach(x =>
    {
        x.ForEach(y =>
        {
            EmpComp empComp = new EmpComp();
            empComp.EmpCompId = y.CompId;
            empComp.EmpCompName = y.CompName;
            empComp.GroupId = y.GroupId ;
            empComp.EmpId = empId + 1;
            empCompList.Add(empComp);
        });
        empId++;
    });

I want to avoid usage of two ForEachs here.

Note: I have some other ids and strings which needs to be assigned based in the GroupId . empId is only an example

Upvotes: 4

Views: 154

Answers (3)

Enigmativity
Enigmativity

Reputation: 117057

There's no need for nested ForEachs or even a SelectMany as the group that is created is just flatten by the nested ForEachs. In fact the current code is incorrect as the DisplayOrder property isn't really ordering when a GroupBy is called - it just happens that the first value for each distinct DisplayOrder is in order in the source data. If it were not the the result would be out of order.

This works without any grouping or use of ForEach, and it correctly orders the result:

List<EmpComp> empCompList =
    compList
        .OrderBy(u => u.DisplayOrder)
        .Select((y, n) => new EmpComp()
        {
            EmpCompId = y.CompId,
            EmpCompName = y.CompName,
            DisplayOrder = y.DisplayOrder,
            EmpId = n + 1,
        }).ToList();

Thanks to the comments I realized the original code had the empId++; outside the inner loop.

Here's my code that fixes the issue:

List<EmpComp> empCompList =
    compList
        .OrderBy(u => u.DisplayOrder)
        .GroupBy(u => u.DisplayOrder)
        .Select((ys, n) =>
            ys
                .Select(y => new EmpComp()
                {
                    EmpCompId = y.CompId,
                    EmpCompName = y.CompName,
                    DisplayOrder = y.DisplayOrder,
                    EmpId = n + 1,
                }))
        .SelectMany(x => x)
        .ToList();

This gives this result:

empCompList

Upvotes: 3

Ivan Stoev
Ivan Stoev

Reputation: 205589

Here is the equivalent of your code using just LINQ

var empCompList = compList
    .GroupBy(c => c.GroupId)
    .SelectMany((g, i) => g.Select(c => new EmpComp
    {
        EmpCompId = c.CompId,
        EmpCompName = c.CompName,
        GroupId = c.GroupId,
        EmpId = i + 1
    }))
    .ToList();

The key point is using the SelectMany overload which receives source element and index.

Upvotes: 1

Tamas Ionut
Tamas Ionut

Reputation: 4410

How about like this (tuned Enigmativity's answer):

int empId=0;
Dictionary<string, int> displayOrderTable = compList.GroupBy(u => u.DisplayOrder).Distinct().ToDictionary(x=>x.Key, (v,k)=>empId++;)
List<EmpComp> empCompList =
    compList
        .OrderBy(u => u.DisplayOrder)
        .Select((y, n) => new EmpComp()
        {
            EmpCompId = y.CompId,
            EmpCompName = y.CompName,
            DisplayOrder = y.DisplayOrder,
            EmpId = displayOrderTable[y.DisplayOrder],
        }).ToList();

This will run in O(N+N) = O(N) time as opposed to O(N^2) (using for in for).

Upvotes: 1

Related Questions