Generic DbDataReader to List<T> mapping

Question

I am having a slight issue (more like an annoyance) with my property binding data access classes. The problem is that the mapping fails when there exists no column in the reader for corresponding property in class.

Code

Here is the mapper class:

// Map our datareader object to a strongly typed list
private static IList<T> Map<T>(DbDataReader dr) where T : new()
{
    try
    {
        // initialize our returnable list
        List<T> list = new List<T>();
        // fire up the lamda mapping
        var converter = new Converter<T>();
        while (dr.Read())
        {
            // read in each row, and properly map it to our T object
            var obj = converter.CreateItemFromRow(dr);
            // add it to our list
            list.Add(obj);
        }
        // reutrn it
        return list;
    }
    catch (Exception ex)
    {    
        return default(List<T>);
    }
}

Converter class:

/// <summary>
/// Converter class to convert returned Sql Records to strongly typed classes
/// </summary>
/// <typeparam name="T">Type of the object we'll convert too</typeparam>
internal class Converter<T> where T : new()
{
    // Concurrent Dictionay objects
    private static ConcurrentDictionary<Type, object> _convertActionMap = new ConcurrentDictionary<Type, object>();
    // Delegate action declaration
    private Action<IDataReader, T> _convertAction;

    // Build our mapping based on the properties in the class/type we've passed in to the class
    private static Action<IDataReader, T> GetMapFunc()
    {
        var exps = new List<Expression>();
        var paramExp = Expression.Parameter(typeof(IDataReader), "o7thDR");
        var targetExp = Expression.Parameter(typeof(T), "o7thTarget");
        var getPropInfo = typeof(IDataRecord).GetProperty("Item", new[] { typeof(string) });
        var _props = typeof(T).GetProperties();
        foreach (var property in _props)
        {

            var getPropExp = Expression.MakeIndex(paramExp, getPropInfo, new[] { Expression.Constant(property.Name, typeof(string)) });
            var castExp = Expression.TypeAs(getPropExp, property.PropertyType);
            var bindExp = Expression.Assign(Expression.Property(targetExp, property), castExp);
            exps.Add(bindExp);

        }
        // return our compiled mapping, this will ensure it is cached to use through our record looping
        return Expression.Lambda<Action<IDataReader, T>>(Expression.Block(exps), new[] { paramExp, targetExp }).Compile();
    }

    internal Converter()
    {
        // Fire off our mapping functionality
        _convertAction = (Action<IDataReader, T>)_convertActionMap.GetOrAdd(typeof(T), (t) => GetMapFunc());
    }

    internal T CreateItemFromRow(IDataReader dataReader)
    {
        T result = new T();
        _convertAction(dataReader, result);
        return result;
    }
}

Exception

System.IndexOutOfRangeException {"Mileage"}

Stacktrace

at System.Data.ProviderBase.FieldNameLookup.GetOrdinal(String fieldName)
at System.Data.SqlClient.SqlDataReader.GetOrdinal(String name)
at System.Data.SqlClient.SqlDataReader.get_Item(String name)
at lambda_method(Closure , IDataReader , Typing )
at o7th.Class.Library.Data.Converter`1.CreateItemFromRow(IDataReader dataReader) in d:\Backup Folder\Development\o7th Web Design\o7th.Class.Library.C-Sharp\o7th.Class.Library\Data Access Object\Converter.cs:line 50
at o7th.Class.Library.Data.Wrapper.Map[T](DbDataReader dr) in d:\Backup Folder\Development\o7th Web Design\o7th.Class.Library.C-Sharp\o7th.Class.Library\Data Access Object\Wrapper.cs:line 33

Question

How can I fix it, so that it will not fail when I have an extra property that the reader may not have as column and vice versa? Of course the quick band-aid would be to simply add NULL As Mileage to this query in example, however, this is not a solution to the problem :)

---

Here's Map<T> using reflection:

// Map our datareader object to a strongly typed list
private static IList<T> Map<T>(DbDataReader dr) where T : new()
{
    try
    {
        // initialize our returnable list
        List<T> list = new List<T>();
        T item = new T();
        PropertyInfo[] properties = (item.GetType()).GetProperties();
        while (dr.Read()) {
            int fc = dr.FieldCount;
            for (int j = 0; j < fc; ++j) {
                var pn = properties[j].Name;
                var gn = dr.GetName(j);
                if (gn == pn) {
                    properties[j].SetValue(item, dr[j], null);
                }
            }
            list.Add(item);
        }
        // return it
        return list;
    }
    catch (Exception ex)
    {
        // Catch an exception if any, an write it out to our logging mechanism, in addition to adding it our returnable message property
        _Msg += "Wrapper.Map Exception: " + ex.Message;
        ErrorReporting.WriteEm.WriteItem(ex, "o7th.Class.Library.Data.Wrapper.Map", _Msg);
        // make sure this method returns a default List
        return default(List<T>);
    }
}

Note: This method is 63% slower than using expression trees...

Answer 1

As noted in comments, the problem is that there exists no column in the reader for the specified property. The idea is to loop by the column names of reader first, and check to see if matching property exists. But how do one get the list of column names beforehand?

1. One idea is to use expression trees itself to build the list of column names from the reader and check it against properties of the class. Something like this

   var paramExp = Expression.Parameter(typeof(IDataRecord), "o7thDR");
   
   var loopIncrementVariableExp = Expression.Parameter(typeof(int), "i");
   var columnNamesExp = Expression.Parameter(typeof(List<string>), "columnNames");
   
   var columnCountExp = Expression.Property(paramExp, "FieldCount");
   var getColumnNameExp = Expression.Call(paramExp, "GetName", Type.EmptyTypes, 
       Expression.PostIncrementAssign(loopIncrementVariableExp));
   var addToListExp = Expression.Call(columnNamesExp, "Add", Type.EmptyTypes, 
       getColumnNameExp);
   var labelExp = Expression.Label(columnNamesExp.Type);
   
   var getColumnNamesExp = Expression.Block(
       new[] { loopIncrementVariableExp, columnNamesExp },
       Expression.Assign(columnNamesExp, Expression.New(columnNamesExp.Type)),
       Expression.Loop(
           Expression.IfThenElse(
               Expression.LessThan(loopIncrementVariableExp, columnCountExp),
               addToListExp,
               Expression.Break(labelExp, columnNamesExp)),
           labelExp));
   

   would be the equivalent of

   List<string> columnNames = new List<string>();
   for (int i = 0; i < reader.FieldCount; i++)
   {
       columnNames.Add(reader.GetName(i));
   }
   

   One may continue with the final expression, but there is a catch here making any further effort along this line futile. The above expression tree will be fetching the column names every time the final delegate is called which in your case is for every object creation, which is against the spirit of your requirement.

2. Another approach is to let the converter class have a pre-defined awareness of the column names for a given type, by means of attributes (see for an example) or by maintaining a static dictionary like (Dictionary<Type, IEnumerable<string>>). Though it gives more flexibility, the flip side is that your query need not always include all the column names of a table, and any reader[notInTheQueryButOnlyInTheTableColumn] would result in exception.

3. The best approach as I see is to fetch the column names from the reader object, but only once. I would re-write the thing like:

   private static List<string> columnNames;
   
   private static Action<IDataReader, T> GetMapFunc()
   {
       var exps = new List<Expression>();
   
       var paramExp = Expression.Parameter(typeof(IDataRecord), "o7thDR");
       var targetExp = Expression.Parameter(typeof(T), "o7thTarget");
   
       var getPropInfo = typeof(IDataRecord).GetProperty("Item", new[] { typeof(string) });
   
       foreach (var columnName in columnNames)
       {
           var property = typeof(T).GetProperty(columnName);
           if (property == null)
               continue;
   
           // use 'columnName' instead of 'property.Name' to speed up reader lookups
           //in case of certain readers.
           var columnNameExp = Expression.Constant(columnName);
           var getPropExp = Expression.MakeIndex(
               paramExp, getPropInfo, new[] { columnNameExp });
           var castExp = Expression.TypeAs(getPropExp, property.PropertyType);
           var bindExp = Expression.Assign(
               Expression.Property(targetExp, property), castExp);
           exps.Add(bindExp);
       }
   
       return Expression.Lambda<Action<IDataReader, T>>(
           Expression.Block(exps), paramExp, targetExp).Compile();
   }
   
   internal T CreateItemFromRow(IDataReader dataReader)
   {
       if (columnNames == null)
       {
           columnNames = Enumerable.Range(0, dataReader.FieldCount)
                                   .Select(x => dataReader.GetName(x))
                                   .ToList();
           _convertAction = (Action<IDataReader, T>)_convertActionMap.GetOrAdd(
               typeof(T), (t) => GetMapFunc());
       }
   
       T result = new T();
       _convertAction(dataReader, result);
       return result;
   }
   

   Now that begs the question why not pass the data reader directly to constructor? That would be better.

   private IDataReader dataReader;
   
   private Action<IDataReader, T> GetMapFunc()
   {
       var exps = new List<Expression>();
   
       var paramExp = Expression.Parameter(typeof(IDataRecord), "o7thDR");
       var targetExp = Expression.Parameter(typeof(T), "o7thTarget");
   
       var getPropInfo = typeof(IDataRecord).GetProperty("Item", new[] { typeof(string) });
   
       var columnNames = Enumerable.Range(0, dataReader.FieldCount)
                                   .Select(x => dataReader.GetName(x));
       foreach (var columnName in columnNames)
       {
           var property = typeof(T).GetProperty(columnName);
           if (property == null)
               continue;
   
           // use 'columnName' instead of 'property.Name' to speed up reader lookups
           //in case of certain readers.
           var columnNameExp = Expression.Constant(columnName);
           var getPropExp = Expression.MakeIndex(
               paramExp, getPropInfo, new[] { columnNameExp });
           var castExp = Expression.TypeAs(getPropExp, property.PropertyType);
           var bindExp = Expression.Assign(
               Expression.Property(targetExp, property), castExp);
           exps.Add(bindExp);
       }
   
       return Expression.Lambda<Action<IDataReader, T>>(
           Expression.Block(exps), paramExp, targetExp).Compile();
   }
   
   internal Converter(IDataReader dataReader)
   {
       this.dataReader = dataReader;
       _convertAction = (Action<IDataReader, T>)_convertActionMap.GetOrAdd(
           typeof(T), (t) => GetMapFunc());
   }
   
   internal T CreateItemFromRow()
   {
       T result = new T();
       _convertAction(dataReader, result);
       return result;
   }
   

   Call it like

   List<T> list = new List<T>();
   var converter = new Converter<T>(dr);
   while (dr.Read())
   {
       var obj = converter.CreateItemFromRow();
       list.Add(obj);
   }
   

---

There are a number of improvements that I can suggest, though.

1. The generic new T() you're calling in CreateItemFromRow is slower, it uses reflection behind the scenes. You can delegate that part to expression trees as well which should be faster

2. Right now GetProperty call isn't case insensitive, meaning your column names will have to exactly match the property name. I would make it case insensitive using one of those Bindings.Flag.

3. I'm not sure at all why you are using a ConcurrentDictionary as a caching mechanism here. A static field in a generic class <T> will be unique for every T. The generic field itself can act as cache. Also why is the Value part of ConcurrentDictionary of type object?

4. As I said earlier, it's not the best to strongly tie a type and the column names (which you're doing by caching one particular Action delegate per type). Even for the same type your queries can be different selecting different set of columns. It's best to leave it to data reader to decide.

5. Use Expression.Convert instead of Expression.TypeAs for value type conversion from object.

6. Also note that reader.GetOrdinal is much faster way to perform data reader lookups.

I would re-write the whole thing like:

readonly Func<IDataReader, T> _converter;
readonly IDataReader dataReader;

private Func<IDataReader, T> GetMapFunc()
{
    var exps = new List<Expression>();

    var paramExp = Expression.Parameter(typeof(IDataRecord), "o7thDR");

    var targetExp = Expression.Variable(typeof(T));
    exps.Add(Expression.Assign(targetExp, Expression.New(targetExp.Type)));

    //does int based lookup
    var indexerInfo = typeof(IDataRecord).GetProperty("Item", new[] { typeof(int) });

    var columnNames = Enumerable.Range(0, dataReader.FieldCount)
                                .Select(i => new { i, name = dataReader.GetName(i) });
    foreach (var column in columnNames)
    {
        var property = targetExp.Type.GetProperty(
            column.name,
            BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase);
        if (property == null)
            continue;

        var columnNameExp = Expression.Constant(column.i);
        var propertyExp = Expression.MakeIndex(
            paramExp, indexerInfo, new[] { columnNameExp });
        var convertExp = Expression.Convert(propertyExp, property.PropertyType);
        var bindExp = Expression.Assign(
            Expression.Property(targetExp, property), convertExp);
        exps.Add(bindExp);
    }

    exps.Add(targetExp);
    return Expression.Lambda<Func<IDataReader, T>>(
        Expression.Block(new[] { targetExp }, exps), paramExp).Compile();
}

internal Converter(IDataReader dataReader)
{
    this.dataReader = dataReader;
    _converter = GetMapFunc();
}

internal T CreateItemFromRow()
{
    return _converter(dataReader);
}