Optimize a generic foreach method that converts Datatable to my object using Reflection I need to optimize this code block below, this method converts a Datatable object to the Object that I am passing by parameter, in another words, this serializes the object, however I have to run this block 1 million times when reading data from the database, so this is the bottleneck of my application.


  Obs: All my objects that I use in this method have a property "Id"
  which is the PrimaryKey of the table, but the name of the primary key
  in the database is different, so that's why I have that "if command"
  asking the name of the primary key.


How can I optimize this code below?

public static List&lt;T&gt; BindDataList&lt;T&gt;(DataTable dt, string primaryKey)
{
    // Get all columns' name
    var columns = (from DataColumn dc in dt.Columns select dc.ColumnName);

    var properties = typeof(T).GetProperties().ToList();

    var lst = new List&lt;T&gt;();

    foreach (DataRow dr in dt.Rows)
    {              
        // Create object
        var ob = Activator.CreateInstance&lt;T&gt;();

        // Get all properties
        properties.ForEach(propertyInfo =&gt;
        {
            if (columns.Any(s =&gt; s.Equals(propertyInfo.Name, StringComparison.OrdinalIgnoreCase)))
            {
                // Fill the data into the property
                if (!(dr[propertyInfo.Name] is DBNull))
                    propertyInfo.SetValue(ob, dr[propertyInfo.Name]);
            }
            else
            {
                if (propertyInfo.Name.ToUpper().Equals("ID"))
                {
                    // Fill the data into the property
                    if (!(dr[primaryKey] is DBNull))
                    {
                        propertyInfo.SetValue(ob, dr[primaryKey].SafeToInt());
                    }
                }
            }
        });

        lst.Add(ob);
    }
    return lst;
}
A few points tickle:


Why is it a static method?
None of the comments are helpful. Good comments say why, not what. I'd remove them all.
Bracing is inconsistent. Consider:

if (!(dr[propertyInfo.Name] is DBNull))
    propertyInfo.SetValue(ob, dr[propertyInfo.Name]);


And then:

if (!(dr[primaryKey] is DBNull))
{
    propertyInfo.SetValue(ob, dr[primaryKey].SafeToInt());
}


I prefer the latter, as explicit scopes are always less error-prone.
Naming is also inconsistent. You call a PropertyInfo, propertyInfo (♥++)... but then you have a List&lt;T&gt; that you call lst instead of list - although since that's the method's return value I'd just call it result.
Return value should probably be exposed as an IEnumerable&lt;T&gt; - it's your method's job to add items to that list, not the client code's. Right?

If you can return an IEnumerable&lt;T&gt;, then you can yield return ob, and drop the lst altogether.

Instead of using reflection to create an instance of T, I would use a : new() generic type constraint to ensure T has a parameterless constructor, and create the instance like this instead:

var ob = new T();

There is no generic type constraint at all, so technically this call would be valid:

var foo = WhateverTheClassIs.BindDataList&lt;int&gt;(dt, "Id");


I would consider changing the signature to include a class generic type constraint that ensures T is a reference type:

public static List&lt;T&gt; BindDataList&lt;T&gt;(DataTable dt, string primaryKey)
    where T : class, new()

ob is an awful name. I understand the intent was to call it object and that would have clashed with System.Object - I would rather see @object than ob, to tell you the truth. But instance is probably a better fit.
dr could be called row instead.