I wouldn't say, your major concern should be performance. I find the method rather error prone and it has a very limited use:
1) All columns have to be of type string
- except the first which must be of type Guid
2) The order of the columns in the table must match the order of the properties of the object type. If they don't, a column could easily be mapped with a wrong property without no warning because they must all be strings. You definitely need some type checking.
3) A data table usually has a (unique) Id column, which the instances of the class should be instantiated with, but it seems, that you create a new object Id for every new instance/row. Is that Id saved to the database? And what about the next time the Row/Item
are mapped - is a new Id again created for that item? It seems not logical to have a new id for an item every time it is fetched from a database?
I would say that the method is so aimed at a special situation where you need to map a data table with a certain class
that it is not a candidate as an extension method - but just as a private method in a class. An extension method should be generic as much as possible and should handle all possible errors in a consistent way.
What about private vs. public
members?
I personally don't like the use of an indexed iteration here: I would go along these lines:
PropertyInfo[] pis = typeof(T).GetProperties();
foreach (DataRow row in dt.Rows)
{
T item = new T();
foreach (DataColumn column in dt.Columns)
{
PropertyInfo pi = pis.FirstOrDefault(p => p.Name == column.ColumnName);
if (pi != null)
{
pi.SetValue(item, row[column]);
}
}
}
or
PropertyInfo[] pis = typeof(T).GetProperties();
foreach (DataRow row in dt.Rows)
{
T item = new T();
foreach (PropertyInfo pi in pis)
{
pi.SetValue(item, row[dt.Columns[pi.Name]]);
}
}
Notice, that the call to typeof(T).GetProperties()
is outside the loops, because there is no need to call it for every row/item
. In this way you are half the way to a nearly completely generic extension method.
In order to make it more generic, you'll have to consider the mapping strategy:
1) Should the column names match the property names of the class or should you consider to be able to provide a mapping table?:
public static List<T> DataTableToList<T>(this DataTable dt, Dictionary<string, string> mappings) where T : class, new()
or:
public static List<T> DataTableToList<T>(this DataTable dt, (string column, string property)[] mappings) where T : class, new()
You'll also have have to consider conversion of certain types: for instance is DateTime
often stored in a data table as SqlDateTime
which can not implicitly cast to a DateTime
structure, so you should maybe be able to provide some converters:
public static List<T> DataTableToList<T>(this DataTable dt, (string column, string property)[] mappings, Dictionary<string, Func<object, object>> converters) where T : class, new()
called like:
Dictionary<string, Func<object, object>> converters = new Dictionary<string, Func<object, object>>
{
{ "BirthDay", (sqlDate) => ((SqlDateTime)sqlDate).Value }
};
List<MyItem> items = table.DataTableToList<MyItem>(null, converters);
Both the mapping and converter strategy can probably be more elegant or sophisticated - the above is merely some quick suggestions.