Making a list of customers who buy the top N items from a supplier

Question

I have a method that works perfectly well and does what it's supposed to, albeit a little slow (30 - 40 seconds). I would like to be able to speed it up or at least make it more efficient, any ideas please?

The purpose is to generate a list of which customers are buying the top requested (topN) lines from a supplier. The first part gets a list of products from the requested supplier, the second part works out which products are the best selling by distribution and then sales and then adds those products to an array and they form the column names for the datatable. The last part loops all of the customer accounts and puts yes or no in the column if they have bought it in the last 3 months. I hope that makes sense.

private void CustomersBuyingRange(string supplier, int topN)
{
    // Get a list of products from a supplier.
    string[] products = productDetails
                        .Where(x => x.SupplierID == supplier.ToUpper())
                        .Select(x => x.ProductCode).ToArray();

    // Rank the products by distribution and then by sales,
    // then add the required amount to an array.
    string[] topSellers = detailedOrderLines
                            .Where(x => products.Contains(x.ProductCode))
                            .GroupBy(x => x.ProductCode)
                            .Select(x => new
                            {
                                x.FirstOrDefault().ProductCode,
                                DeliveredQty = x.Sum(p => p.DeliveredQty),
                                Distribution = x.Select(c => c.CustomerID).Distinct().Count()
                            })
                            .OrderByDescending(x => x.Distribution)
                            .ThenByDescending(x => x.DeliveredQty)
                            .Take(topN)
                            .Select(a => a.ProductCode).ToArray();

    DataTable table = new DataTable("Customers");
    table.Columns.Add("CustomerID", typeof(string));
    table.Columns.Add("Customer", typeof(string));
    table.Columns.Add("AccountManager", typeof(string));
    // Add the columns with the products.
    foreach (string product in topSellers)
    {
        table.Columns.Add(product, typeof(string));
    }

    // We only want customers that have had an order in the last 3 months.
    var customers = customerDetails
        .Where(x => x.LastInvoiceDate > DateTime.Now.AddMonths(-3).Date)
        .OrderBy(x => x.CustomerName)
        .ToList();

    foreach (var customer in customers)
    {
        DataRow row = table.NewRow();
        row["CustomerID"] = customer.CustomerID;
        row["Customer"] = customer.CustomerName;
        row["AccountManager"] = customer.AccountManager;
        for (int i = 0; i < topN; i++)
        {
            row[topSellers[i]] = detailedOrderLines
                .Any(x => x.CustomerID == customer.CustomerID &&
                     x.ProductCode == topSellers[i] &&
                     x.InvoiceDate > DateTime.Now.AddMonths(-3).Date
                     ) ? "Yes" : "No";
        }

        table.Rows.Add(row);
    }
    Export.ExportToExcel(table, true);
}

There are roughly 3000 customers.
Any advise would be greatly appreciated.

Answer 1

As said in a comment this may be done better and faster with an SQL script depending on how the involved data sets are created, but below are some optimization suggestions.

---

You should maybe consider some input check:

private void CustomersBuyingRange(string supplier, int topN)
{
  if (string.IsNullOrWhiteSpace(supplier)) return;
  if (topN <= 0) throw new ArgumentOutOfRangeException(nameof(topN));

---

Optimizations:

1)

  // Get a list of products from a supplier.
  string[] products = productDetails
                      .Where(x => x.SupplierID == supplier.ToUpper())
                      .Select(x => x.ProductCode).ToArray();

supplier.ToUpper() is called repeatedly for every element in the vector. Consider to do it once:

  supplier = supplier.ToUpper();

  // Get a list of products from a supplier.
  var products = productDetails
                      .Where(x => x.SupplierID == supplier)
                      .Select(x => x.ProductCode);

As shown, there is also no need to populate an array with the result here.

---

2)

  var customers = customerDetails
      .Where(x => x.LastInvoiceDate > DateTime.Now.AddMonths(-3).Date)
      .OrderBy(x => x.CustomerName)
      .ToList();

Again you create a new DateTime object for each customer you evaluate in the Where() call. Consider to create that date once:

    DateTime minDate = DateTime.Now.AddMonths(-3).Date;
    // We only want customers that have had an order in the last 3 months.
    var customers = customerDetails
        .Where(x => x.LastInvoiceDate > minDate)
        .OrderBy(x => x.CustomerName);

You can then reuse it in the last loop too.

---

3)

    for (int i = 0; i < topN; i++)
    {
      row[topSellers[i]] = detailedOrderLines
          .Any(x => x.CustomerID == customer.CustomerID &&
               x.ProductCode == topSellers[i] &&
               x.InvoiceDate > DateTime.Now.AddMonths(-3).Date
               ) ? "Yes" : "No";
    }

Here you should be able to run a foreach loop instead:

      foreach (var topSeller in topSellers)
      {
        row[topSeller] = detailedOrderLines
            .Any(x => x.CustomerID == customer.CustomerID &&
                 x.ProductCode == topSeller &&
                 x.InvoiceDate > minDate) ? "Yes" : "No";
      }

---

4)

foreach (var customer in customers)
  {
    DataRow row = table.NewRow();
    row["CustomerID"] = customer.CustomerID;
    row["Customer"] = customer.CustomerName;
    row["AccountManager"] = customer.AccountManager;
    for (int i = 0; i < topN; i++)
    {
      row[topSellers[i]] = detailedOrderLines
          .Any(x => x.CustomerID == customer.CustomerID &&
               x.ProductCode == topSellers[i] &&
               x.InvoiceDate > DateTime.Now.AddMonths(-3).Date
               ) ? "Yes" : "No";
    }
    table.Rows.Add(row);
  }

Here you keep require the entire detailedOrderLines although you previously when creating the topSellers actually grouped those by ProductCode. So if you instead of selecting topSellers as strings select them as anonymous objects like:

  // Rank the products by distribution and then by sales,
  // then add the required amount to an array.
  var topProducts = detailedOrderLines
                          .Where(x => products.Contains(x.ProductCode))
                          .GroupBy(x => x.ProductCode)
                          .Select(x => new
                          {
                            ProductCode = x.FirstOrDefault().ProductCode,
                            DeliveredQty = x.Sum(p => p.DeliveredQty),
                            Distribution = x.Select(c => c.CustomerID).Distinct().Count(),
                            OrderLines = x.ToList()
                          })
                          .OrderByDescending(x => x.Distribution)
                          .ThenByDescending(x => x.DeliveredQty)
                          .Take(topN).ToList();

you only have to query the subset of orderLines that belongs to the current ProductCode:

    foreach (var customer in customers)
    {
      DataRow row = table.NewRow();
      row["CustomerID"] = customer.CustomerID;
      row["Customer"] = customer.CustomerName;
      row["AccountManager"] = customer.AccountManager;

      foreach (var topProduct in topProducts)
      {
        row[topProduct.ProductCode] = topProduct.OrderLines.Any(x => x.CustomerID == customer.CustomerID && x.InvoiceDate < minDate) ? "Yes" : "No";
      }

      table.Rows.Add(row);
    }

---

All in all the method could then be something like:

private void CustomersBuyingRange(string supplier, int topN)
{
  if (string.IsNullOrWhiteSpace(supplier)) return;
  if (topN <= 0) throw new ArgumentOutOfRangeException(nameof(topN));

  supplier = supplier.ToUpper();

  // Get a list of products from a supplier.
  var products = productDetails
                      .Where(x => x.SupplierID == supplier)
                      .Select(x => x.ProductCode);

  // Rank the products by distribution and then by sales,
  // then add the required amount to an array.
  var topProducts = detailedOrderLines
                          .Where(x => products.Contains(x.ProductCode))
                          .GroupBy(x => x.ProductCode)
                          .Select(x => new
                          {
                            ProductCode = x.FirstOrDefault().ProductCode,
                            DeliveredQty = x.Sum(p => p.DeliveredQty),
                            Distribution = x.Select(c => c.CustomerID).Distinct().Count(),
                            OrderLines = x.ToList()
                          })
                          .OrderByDescending(x => x.Distribution)
                          .ThenByDescending(x => x.DeliveredQty)
                          .Take(topN).ToList();

  using (DataTable table = new DataTable("Customers"))
  {
    table.Columns.Add("CustomerID", typeof(string));
    table.Columns.Add("Customer", typeof(string));
    table.Columns.Add("AccountManager", typeof(string));
    // Add the columns with the products.
    foreach (var topProduct in topProducts)
    {
      table.Columns.Add(topProduct.ProductCode, typeof(string));
    }

    DateTime minDate = DateTime.Now.AddMonths(-3).Date;
    // We only want customers that have had an order in the last 3 months.
    var customers = customerDetails
        .Where(x => x.LastInvoiceDate > minDate)
        .OrderBy(x => x.CustomerName);


    foreach (var customer in customers)
    {
      DataRow row = table.NewRow();
      row["CustomerID"] = customer.CustomerID;
      row["Customer"] = customer.CustomerName;
      row["AccountManager"] = customer.AccountManager;

      foreach (var topProduct in topProducts)
      {
        row[topProduct.ProductCode] = topProduct.OrderLines.Any(x => x.CustomerID == customer.CustomerID && x.InvoiceDate < minDate) ? "Yes" : "No";
      }

      table.Rows.Add(row);
    }

    Export.ExportToExcel(table, true);
  }
}

---

Disclaimer: I haven't had the opportunity to test the above, so don't hang me if it doesn't improve anything or if I have misunderstood something.

Answer 2

• My eye started twitching uncontrollably when I saw .Where(x => x.SupplierID == supplier.ToUpper()). To me, this indicates a major design flaw: something that's called an ID should not be compared to a string that needs to be uppercased.

---

• Names are important. string[] products = [...] .Select(x => x.ProductCode).ToArray(); suggests to me that this should be named productCodes, not products. See also DataTable table: "table" is about as undescriptive as you can get.

---

• The best solution would be to investigate if you couldn't replace all this code with a single query.