CODEWITHSUNDEEP
c#multithreadingperformancefor-loopforeach
Abhishek Thakur
Abhishek Thakur

Reputation: 21

What can I do to make this foreach loop run faster as it takes long time to execute?

What can I do to make this loop run faster?

    private void accessVendorGridData()
    {
        try
        {
            foreach (var item in getAllVendorList)
            {
                item.CurrencyName = "USD";

                // Fetch Addresses in Vendor Grid
                var Addr = _vendorservice.GetAllVendorAdd().Where(x => x.vendorId == item.Id).ToList();
                if (Addr.Count > 0)
                {
                    item.VendorAddressLine = String.Format("{0}, {1}, {2}, {3}, {4}", Addr[0].Address, Addr[0].City, Addr[0].StateProvince, Addr[0].ZipPostalCode, Addr[0].CountryRegion);
                }

                // Fetch Payment terms in Vendor Grid
                var paymentTerm = _vendorservice.GetAllPaymentTerms().Where(x => x.Id == item.PaymentTermId).ToList().SingleOrDefault();
                if (paymentTerm != null)
                {
                    item.paymenttermitem = paymentTerm.Name;
                }

                // Fetch Tax Scheme in Vendor Grid
                var taxscheme = _vendorservice.GetAllTaxScheme().Where(x => x.Id == item.TaxschemeId).ToList().SingleOrDefault();
                if (taxscheme != null)
                {
                    item.TaxschemeName = taxscheme.TaxSchemaName;
                }
            }
        }
        catch (Exception ex)
        {
            _exLog.AddErrorLog(ex, "NewVendor, accessVendorGridData()");
            ModernDialog.ShowMessage(ex.Message, "Error!", MessageBoxButton.OK);
        }
    }

What can I do to make this loop run faster? I tried Parallel.ForEach but lost in between. Can somebody help?

private void accessVendorGridData()
    {
            foreach (var item in getAllVendorList)
            {
                var Addr = _vendorservice.GetAllVendorAdd().Where(x => x.vendorId == item.Id).ToList();
                var paymentTerm = _vendorservice.GetAllPaymentTerms().Where(x => x.Id == item.PaymentTermId).ToList().SingleOrDefault();
                var taxscheme = _vendorservice.GetAllTaxScheme().Where(x => x.Id == item.TaxschemeId).ToList().SingleOrDefault();
            }
    }

Upvotes: 0

Views: 105

Answers (1)

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186668

Instead of querying at each iteration you can extract vendors, paymentTerms and allTaxSchemes from the loop as dictionaries:

private void accessVendorGridData() {
  var vendors = _vendorservice
    .GetAllVendorAdd()  
    .GroupBy(item => item.Id)
    .ToDictionary(chunk => chunk.Key, chunk => chunk.ToList());

  var paymentTerms = _vendorservice
    .GetAllPaymentTerms()
    .GroupBy(item => item.Id)
    .ToDictionary(chunk => chunk.Key, chunk => chunk.SingleOrDefault());

  var allTaxSchemes = _vendorservice
    .GetAllTaxScheme()
    .GroupBy(item => item.Id)
    .ToDictionary(chunk => chunk.Key, chunk => chunk.SingleOrDefault());

  foreach (var item in getAllVendorList) {
    var Addr = vendors.TryGetValue(item.Id, out var addrs) 
       ? addrs 
       : new List<Vendor>(); //TODO: put the right type instead of Vendor

    var paymentTerm = paymentTerms.TryGetValue(item.PaymentTermId, out var term) 
       ? term 
       : null;

    var taxscheme = allTaxSchemes.TryGetValue(item.PaymentTermId, out var scheme) 
       ? scheme 
       : null;
  } 
}

Your current code has

O(|getAllVendorList| * (|vendors| + |paymentTerms| + |allTaxSchemes|))

time complexity, this one has

O(|getAllVendorList| + |vendors| + |paymentTerms| + |allTaxSchemes|)

However, it's not a solution if _vendorservice.GetXXX() is a query to service, RDBMS etc. and you have to call it at each iteration (since data can be changed)

Upvotes: 2

Related Questions