What is the alternate way of using LINQ ForEach loop?

Question

 medicineList.ForEach(x =>
                         {
                             DoctorsOrderViewModel vm = new DoctorsOrderViewModel()
                             {
                                 DrugID = x.PKID,
                                 Name = x.Name,
                                 DrugName = x.Name,
                                 UnitName = x.UnitName,
                                 CategoryID = x.CategoryID,
                                 CategoryName = x.CategoryName,
                                 DosageFormID = x.DosageFormID,
                                 InventoryTypeID = x.InventoryTypeID,
                             };

                             temp.Add(vm);
                             this.DrugItemsComboForSearch.Add(vm);


                             DoctorsOrderViewModel vm2 = new DoctorsOrderViewModel() { CategoryID = x.CategoryID, CategoryName = x.CategoryName, };

                             if (!this.MedicineCategoryItemsCombo.Select(y => y.CategoryID).Contains(x.CategoryID))
                             {
                                 this.MedicineCategoryItemsCombo.Add(vm2);
                             }
                         });

In my Case for 13000 Medicine this code took 8-10 sec to complete but its too lengthy considering performance issue. How can i optimized this?

Linq does not have a ForEach function. ForEach is just a normal method on the List<T> class. — Scott Chamberlain
– Scott Chamberlain, Commented Sep 5, 2018 at 3:46
@Dishant He's adding things in the loop - concurrency-safe lists would have to be used. In this case it would be an overkill — tmaj
– tmaj, Commented Sep 5, 2018 at 6:36

Scott Chamberlain · Accepted Answer · 2018-09-05 03:53:21Z

4

What is the alternate way of using LINQ ForEach loop?

A standard foreach.

How can i optimized this

As to performance, its not yourForEach that's the problem, its probably the select and contains ,consider using a ToHashSet once

var set = this.MedicineCategoryItemsCombo.Select(y => y.CategoryID).ToHashSet();

Then you can use in your loop

if (set.Add(x.CategoryID))
{
     this.MedicineCategoryItemsCombo.Add(vm2);
}

However on reading your code, this can probably be optimised with a better query and Where, then do a Select

edited Sep 5, 2018 at 3:53

Scott Chamberlain

128k37 gold badges299 silver badges448 bronze badges

answered Sep 5, 2018 at 3:43

TheGeneral

82k9 gold badges111 silver badges151 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Z.R.T. Over a year ago

from the author code snipped this.MedicineCategoryItemsCombo.Contains(y => y.CategoryID == x.CategoryID) would be not faster ?

Scott Chamberlain Over a year ago

@Z.R.T. no, a HashSet would be faster. The primary use case for HashSets is to do extremely fast contains checks.

TheGeneral Over a year ago

Z.R.T what @ScottChamberlain said

Scott Chamberlain Over a year ago

@TheGeneral I changed your .Contains to a .Add, it still returns a bool but it will add the new entries to the set as you check them keeping MedicineCategoryItemsCombo and set in sync. Also, it would have been if (!set.Contains(x.CategoryID)) for the proper if check and you never added new items to the set so you could end up adding duplicates to the combo. Switching to just Add solves both of those problems.

TheGeneral Over a year ago

@ScottChamberlain if i could up vote you more i would, thanks for the edits!

tmaj · Accepted Answer · 2018-09-05 06:32:58Z

Update: I got some time so I was able to write a complete example:

The results:

10x OPWay for 13000 medicines and 1000 categories: 00:00:03.8986663
10x MyWay for 13000 medicines and 1000 categories: 00:00:00.0879221

Summary

Use AddRange after a transformation with .Select
Use Distinct at the end of the process rather than scanning and adding one by one in each loop.

Solution

    public static List<(string catId, string catName)> MyWay(List<Medicine> medicineList)
    {
        var temp = new List<DoctorsOrderViewModel>();
        var DrugItemsComboForSearch = new List<DoctorsOrderViewModel>();

        var transformed = medicineList.Select(x =>
        {
            return new DoctorsOrderViewModel()
            {
                DrugID = x.PKID,
                Name = x.Name,
                DrugName = x.Name,
                UnitName = x.UnitName,
                CategoryID = x.CategoryID,
                CategoryName = x.CategoryName,
                DosageFormID = x.DosageFormID,
                InventoryTypeID = x.InventoryTypeID,
            };

        }).ToList(); ;

        temp.AddRange(transformed);
        DrugItemsComboForSearch.AddRange(transformed);

        var MedicineCategoryItemsCombo = transformed.Select(m => (catId: m.CategoryID, catName: m.CategoryName)).Distinct().ToList();

        return MedicineCategoryItemsCombo;
    }

Full example:

public static class MainClass
{
    public class Medicine
    {
        public string PKID { get; set; }
        public string Name { get; set; }
        public string UnitName { get; set; }

        public string CategoryID { get; set; }
        public string CategoryName { get; set; }
        public string DosageFormID { get; set; }
        public string InventoryTypeID { get; set; }
    }

    public class DoctorsOrderViewModel
    {
        public string DrugID { get; set; }
        public string Name { get; set; }
        public string DrugName { get; set; }
        public string UnitName { get; set; }

        public string CategoryID { get; set; }
        public string CategoryName { get; set; }
        public string DosageFormID { get; set; }
        public string InventoryTypeID { get; set; }
    }

    class Category
    {
        public string CategoryID { get; set; }
    }

    public static void Main()
    {

        var medicines = new List<Medicine>();

        medicines.AddRange(Enumerable.Range(0, 13000).Select(i => new Medicine()
        {
            PKID = "PKID" + i,
            Name = "Name" + i,
            UnitName = "UnitName" + i,
            CategoryID = "CategoryID" + i%1000,
            CategoryName = "CategoryName for CategoryID" + i%1000,
            DosageFormID = "DosageFormID" + i,
            InventoryTypeID = "InventoryTypeID" + i,
        }));

        Stopwatch sw = new Stopwatch();
        sw.Start();
        List<DoctorsOrderViewModel> comboData = null;
        for (int i = 0; i < 10; i++)
        {
            comboData = OpWay(medicines);
        }
        var elapsed = sw.Elapsed;

        Console.WriteLine($"10x OPWay for {medicines.Count} medicines and {comboData.Count} categories: {elapsed}");


        sw.Restart();
        List<(string catId, string catName)> comboData2 = null;
        for (int i = 0; i < 10; i++)
        {
            comboData2 = MyWay(medicines);
        }
        elapsed = sw.Elapsed;

        Console.WriteLine($"10x MyWay for {medicines.Count} medicines and {comboData2.Count} categories: {elapsed}");

    }

    public static List<DoctorsOrderViewModel> OpWay(List<Medicine> medicineList)
    {
        List<DoctorsOrderViewModel> MedicineCategoryItemsCombo = new List<DoctorsOrderViewModel>();

        var temp = new List<DoctorsOrderViewModel>();
        var DrugItemsComboForSearch = new List<DoctorsOrderViewModel>();

        medicineList.ForEach(x =>
        {
            DoctorsOrderViewModel vm = new DoctorsOrderViewModel()
            {
                DrugID = x.PKID,
                Name = x.Name,
                DrugName = x.Name,
                UnitName = x.UnitName,
                CategoryID = x.CategoryID,
                CategoryName = x.CategoryName,
                DosageFormID = x.DosageFormID,
                InventoryTypeID = x.InventoryTypeID,
            };

            temp.Add(vm);
            DrugItemsComboForSearch.Add(vm);


            DoctorsOrderViewModel vm2 = new DoctorsOrderViewModel() { CategoryID = x.CategoryID, CategoryName = x.CategoryName, };

            if (!MedicineCategoryItemsCombo.Select(y => y.CategoryID).Contains(x.CategoryID))
            {
                MedicineCategoryItemsCombo.Add(vm2);
            }
        });

        return MedicineCategoryItemsCombo;
    }

    public static List<(string catId, string catName)> MyWay(List<Medicine> medicineList)
    {
        var temp = new List<DoctorsOrderViewModel>();
        var DrugItemsComboForSearch = new List<DoctorsOrderViewModel>();

        var transformed = medicineList.Select(x =>
        {
            return new DoctorsOrderViewModel()
            {
                DrugID = x.PKID,
                Name = x.Name,
                DrugName = x.Name,
                UnitName = x.UnitName,
                CategoryID = x.CategoryID,
                CategoryName = x.CategoryName,
                DosageFormID = x.DosageFormID,
                InventoryTypeID = x.InventoryTypeID,
            };

        }).ToList(); ;

        temp.AddRange(transformed);
        DrugItemsComboForSearch.AddRange(transformed);

        var MedicineCategoryItemsCombo = transformed.Select(m => (catId: m.CategoryID, catName: m.CategoryName)).Distinct().ToList();

        return MedicineCategoryItemsCombo;
    }
}

I think this is better, and could be expanded for a complete solution

Samuel Liew · Accepted Answer · 2018-09-19 11:07:08Z

You can use different approach for foreach, which is better than above, also code could be minimised a bit:

foreach (Medicine medicine in medicineList)
            {
                DoctorsOrderViewModel vm = new DoctorsOrderViewModel()
                {
                    DrugID = x.PKID,
                    Name = x.Name,
                    DrugName = x.Name,
                    UnitName = x.UnitName,
                    CategoryID = x.CategoryID,
                    CategoryName = x.CategoryName,
                    DosageFormID = x.DosageFormID,
                    InventoryTypeID = x.InventoryTypeID,
                };

                temp.Add(vm);
                this.DrugItemsComboForSearch.Add(vm);




                if (!this.MedicineCategoryItemsCombo.Select(y => y.CategoryID == 
                    x.CategoryID).Any())
                {
                    this.MedicineCategoryItemsCombo.Add(new DoctorsOrderViewModel()
                    {
                        CategoryID = x.CategoryID,
                        CategoryName = x.CategoryName,
                    };);
                }
            }

Collectives™ on Stack Overflow

What is the alternate way of using LINQ ForEach loop?

3 Answers 3

5 Comments

The results:

Summary

Solution

2 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

The results:

Summary

Solution

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related