Better method for updating data table

Question

I have completed an application which gathers specific records and marks them as Collected. As of now, the application runs perfectly and does exactly what I need it to, but the problem comes in when a large data set is reviewed. If I am pulling records for one day or even maybe a week, it runs at a decent pace, but once you get a month of data or more, it takes a fairly long amount of time.

DataTable ModelData = getModelData() // Returns all records to search.
...
EnumerableRowCollection<DataRow> modelRows = (from model in ModelData.AsEnumerable()
                                              where (model.Field<object>(GeographicalKey) ?? (object)String.Empty).ToString() == GeographicCode
                                              select model);
ModelResults = modelRows.Any() ? modelRows.CopyToDataTable() : ModelData.Clone();

for (int i = 0; i < ModelResults.Rows.Count; i++)
{
    for (int j = 0; j < ModelData.Rows.Count - 1; j++)
    {
        if (ModelResults.Rows[i]["Request ID"].ToString() == ModelData.Rows[j]["Request ID"].ToString())
        {
            ModelData.Rows[j]["Collected"] = "1";
        }
    }
}

The only part that executes slowly would be the for and the nested for. Is there possibly a better way that I can code this? I can't imagine this is the optimal coding for data table update operations, but it might be. Is this a possibility in LINQ (I'm completely new to LINQ)? I think this is enough code to make sense of what I'm doing, but if more is needed just let me know and I can post it up as well.

Answer 1

The only part that executes slowly would be the for and the nested for. Is there possibly a better way that I can code this?

I take you ask us to optimize the nested loops:

The most obvious problem, as @Antonio also noted, is the O(n^2) filtering in the nested fors. Also as he noted, you need some kind of data structure with O(1) Contains method. However since you do not use other fields of from the rows of ModelResults, you only need a HashSet of ["Request ID"]s.

I can't imagine this is the optimal coding for data table update operations, maybe though? Is this a possibility in LINQ (I'm completely new to LINQ)?

I take you are asking for a more compact, more LINQy way of updating a collection, in this case a DataTable:

LINQ is a query language and a LINQ query does not modify underlying IEnumerable. (In most cases you would select a modified copy of the underlying IEnumerable, instead.)

However, you can use List.ForEach on the result of such a query, as a more compact replacement of for loop.

Applying two suggestions above, we get:

var collectedRequestIds = new HashSet<string>(
    modelResults.AsEnumerable().Select(row => row["Request ID"].ToString()));

modelData.AsEnumerable()
    .Where(collectedRequestIds.Contains(row => row["Request ID"].ToString())))
    .ToList().ForEach(row => row["Collected"] = "1");

Answer 2

There are a few optimizations that can be done in your code (such as using a break in the inner loop after a match is found - but reverting the order of the 2 loops) - but leaving the complexity order to O(n^2).

A better approach instead is using a dictionary. Since you have a list of results, identified by a unique ID (Request ID), and a list of rows, uniquely identified in the same way, I would:

• create a dictionary, adding all ModelData rows, using Unique ID as key
• remove the 2nd loop, and just check for an element in the dictionary having the same ID as the ModelData row - if a match is found, set the Collected flag

This should reduce the complexity from O(n^2) to O(n).