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?
-
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.
-
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 anyreader[notInTheQueryButOnlyInTheTableColumn]
would result in exception. -
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.
-
The generic
new T()
you’re calling inCreateItemFromRow
is slower, it uses reflection behind the scenes. You can delegate that part to expression trees as well which should be faster -
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 thoseBindings.Flag
. -
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 everyT
. The generic field itself can act as cache. Also why is theValue
part ofConcurrentDictionary
of typeobject
? -
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. -
Use
Expression.Convert
instead ofExpression.TypeAs
for value type conversion fromobject
. -
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);
}