5
\$\begingroup\$

I need to iterate through a DataTable and columns in each rows and do some data manipulation.

Below given is an extract from my code, I use c#.net. I expect maximum rows in datatable will be less than 25K.

How I can improve the code for better performance?

DataTable metadaDataTable = GetMetaDataTable();
foreach (DataRow row in metadaDataTable.Rows)
{
    string newfileName = string.Empty;
    foreach (DataColumn col in metadaDataTable.Columns)
    {
       if (col.ColumnName != "path_variable")
       {
           newfileName += row[col].ToString() + "_";
       }
     }
       newPath = CleanFileName(newfileName);
   }
\$\endgroup\$
  • \$\begingroup\$ You need to give us waaay more context, e.g. what's the purpose of this code, how many rows, what's the DB structure, etc. Why are you using ADO.NET and not Entity Framework? \$\endgroup\$ – BCdotWEB Jan 10 '17 at 23:17
  • \$\begingroup\$ This is an addition in the erxisting code, so I need to use something which fix in the same class. Maximum rows can be less than 25,000 \$\endgroup\$ – blue Jan 10 '17 at 23:20
3
\$\begingroup\$

One simple trick here would be to take the column processing outside of the row processing (the column metadata should not change while the rows probably will):

using (var metadaDataTable = GetMetaDataTable())
{
    var columns = metadaDataTable.Columns
        .Cast<DataColumn>()
        .Where(col => col.ColumnName != "path_variable")
        .ToList();
    var path_variable = metadaDataTable.Columns
        .Cast<DataColumn>()
        .SingleOrDefault(col => col.ColumnName == "path_variable");
    string path_variable_value;

    foreach (var row in metadaDataTable.Rows.Cast<DataRow>())
    {
        var newfileName = columns.Aggregate(string.Empty, (current, col) => current + (row[col] + "_"));

        newPath = CleanFileName(newfileName);
        path_variable_value = path_variable == null ? null : row[path_variable].ToString();
        // TODO: do something with the path_variable_value before continuing the loop.
    }
}

Note also I've sprinkled a liberal amount of LINQ in there to signify intent rather than mechanism.

Finally, Also surrounded the lifetime of metaDataTable with a using construct as DataTable implements the IDisposable interface.

\$\endgroup\$
  • \$\begingroup\$ Can I include the "path_variable" also in the columns? After getting "newfileName" I need to get the value of "path_variable". \$\endgroup\$ – blue Jan 10 '17 at 23:50
  • \$\begingroup\$ @blue added some quick stuff. \$\endgroup\$ – Jesse C. Slicer Jan 10 '17 at 23:55
  • \$\begingroup\$ How do I exclude any columns with null or empty value in this statement - "var newfileName = columns.Aggregate(string.Empty, (current, col) => current + (row[col] + "_")); " \$\endgroup\$ – blue Jan 11 '17 at 0:28
  • 3
    \$\begingroup\$ I believe that's starting to exceed the scope of this site and you may want to ask such additional question(s) on Stack Overflow. \$\endgroup\$ – Jesse C. Slicer Jan 11 '17 at 0:35
1
\$\begingroup\$

string is immutable
newfileName += row[col].ToString() + "_"; is inefficient

DataTable metadaDataTable = GetMetaDataTable();
StringBuilder sb = new StringBuilder();
foreach (DataRow row in metadaDataTable.Rows)
{
    sb.Clear();
    foreach (DataColumn col in metadaDataTable.Columns)
    {
       if (col.ColumnName != "path_variable")
       {
           sb.Append(row[col].ToString() + "_");
       }
    }
    newPath = CleanFileName(sb.ToString());
}

if you want to get serious

DataTable metadaDataTable = GetMetaDataTable();
int colCount = metadaDataTable.Columns.Count;
int skip = 0;
foreach (DataColumn col in metadaDataTable.Columns)
{
   if (col.ColumnName == "path_variable")
       break;
   skip++;
}
StringBuilder sb = new StringBuilder();
foreach (DataRow row in metadaDataTable.Rows)
{
    sb.Clear();
    for (int i = 0; i < colCount; i++)
    {
       if (i == skip)
           continue;
       sb.Append(row[i].ToString() + "_");
    }
    newPath = CleanFileName(sb.ToString());
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.