Converting recordsets to POCO

Posted on

Problem

The goal this method was to emulate the way EF or Dapper return to you concrete POCO classes instead of recordsets when querying your db. Our current code is riddled with iterations over recordsets and I want to consolidate to an extension method that can convert the recordset into an Enumerable<T>.

Review

I’m concerned about how efficient this will be. When querying the db for a single or handful of records I’m not concerned but we sometimes query for an entire table (i.e. ~100,000 records) and in that case will the iterations in this start to cause performance issues? Is there a better way to resolve the recordsets to classes without iterating? Any other constructive remarks appreciated.

public static IEnumerable<T> AsList<T>(this Recordset rs) where T : new()
{
    IEnumerable<PropertyInfo> props = new List<PropertyInfo>(typeof(T).GetProperties()).Where(p => p.CanWrite);
    List<T> results = new List<T>();
    while (!rs.Eof)
    {
        T obj = new T();
        foreach (PropertyInfo property in props)
        {
            var val = rs[property.Name].Value;
            if (val != null) property.SetValue(obj, Convert.ChangeType(val, property.PropertyType));

        }
        rs.MoveNext();

        results.Add(obj);
    }

    return results.AsEnumerable();
}

used as…

Recordset rs = new Recordset();
rs.Open("select * from Users", connection)
IEnumerable<User> results = rs.AsList<User>();

Post Review Update

After great feedback from Heslacher and t3chb0t the following code has been finalized. Note there was a change in the requirements and sister method was created As<T>() to create a single class instance from a recordset. The feedback from the review has been applied to it as well.

public static IList<T> AsList<T>(this Recordset rs) where T : class, new()
{
    var results = new List<T>();

    if (rs == null) throw new ArgumentNullException("rs");
    if (rs.Eof) return results;

    IList<PropertyInfo> properties = GetProperties<T>();

    while (!rs.Eof)
    {
        results.Add(CreateInstance<T>(properties, rs));
        rs.MoveNext();
    }

    return results;
}


public static T As<T>(this Recordset rs) where T : class, new()
{
    if (rs.Eof) return default(T);
    IList<PropertyInfo> properties = GetProperties<T>();

    return CreateInstance<T>(properties, rs);
}

private static T CreateInstance<T>(IEnumerable<PropertyInfo> properties, Recordset rs) where T : new()
{
    T obj = new T();
    foreach (PropertyInfo property in properties)
    {
        var val = rs[property.Name].Value;
        if (val != null && !Convert.IsDBNull(val)) property.SetValue(obj, Convert.ChangeType(val, property.PropertyType));
    }

    return obj;
}

private static IList<PropertyInfo> GetProperties<T>()
{
    return new List<PropertyInfo>(typeof(T).GetProperties().Where(p => p.CanWrite));
}

Solution

public static IEnumerable<T> AsList<T>(this Recordset rs) where T : new()
{
    ..
    List<T> results = new List<T>();
    ..   
    return results.AsEnumerable();
}

This might be very inefficient because when I see that a method returns an IEnumerable<T> I assume its execution is deferred so I automatically do AsList(..).ToList() to execute it but what it actually does, is to iterate the list twice. First time to create the first list and then to create the second list. You should either create a real deferred method or return the List<T> so there is no doubt about the result.


IEnumerable<PropertyInfo> props = new List<PropertyInfo>(typeof(T).GetProperties()).Where(p => p.CanWrite);

You are executing this Where query for each record because the list wrapps only the GetProperties part (which is an array anyway so I don’t understand why you convert it into a list in the first place – linq can work with arrays). You need to call the ToList extension after the Where

var properties = typeof(T).GetProperties().Where(p => p.CanWrite).ToList();

I also suggest moving the object creation into a separate method so that you don’t have any nested loops like this:

public static IEnumerable<T> AsList<T>(this Recordset rs) where T : new()
{
    var properties = typeof(T).GetProperties().Where(p => p.CanWrite).ToList();
    while (!rs.Eof)
    {
        yield return CreateInstance<T>(properties, rs);        
        rs.MoveNext();
    }
}

private static T CreateInstance<T>(IEnumerable<PropertyInfo> properties, Row row) where T : new()
{
    var obj = new T();
    foreach (var property in properties)
    {
        var value = row[property.Name].Value;
        if (value != null) property.SetValue(obj, Convert.ChangeType(value, property.PropertyType));
    }
    return obj;
}

public static IEnumerable<T> AsList<T>(this Recordset rs) where T : new()  

You are returning an IEnumerable<T> not a List so you should change the name of the method.

  • Because the method is public you should do proper parameter validation.
  • The constaraint new() should be expanded with class so a user of this method would see at first glance that IEnumerable<int> results = rs.AsList<int>(); won’t work. Without the constraint to use this method only with reference types like AsList<int>() you will get for each record in the recordset a value of 0 because the props variable won’t have any items in it.
  • You should use the var type if its type is obvious from the right hand side of the assignment.
  • If your class doesn’t have any writeable properties you should return early.
  • Because you are returning an IEnumerable<T> you could just yield each created obj.
  • You named a single PropertyInfo property but the List<PropertyInfo>you named props. You shouldn’t abbreviate variable names because it makes it harder to grasp at first glance what the variable is about.
  • The mentioned props would be better initialized by using the Where() on the returned values of GetProperties().

Putting most of the mentioned points together (without method name) would lead to

public static IEnumerable<T> AsList<T>(this Recordset rs) where T : class, new()
{
    if (rs == null) { throw new ArgumentNullException("rs"); }
    if (rs.EOF) { return Enumerable.Empty<T>(); }

    var properties = new List<PropertyInfo>(typeof(T).GetProperties().Where(p => p.CanWrite));
    if (properties.Count == 0) { return Enumerable.Empty<T>(); }

    return AsListInternal<T>(rs, properties);

}
private static IEnumerable<T> AsListInternal<T>(Recordset rs, List<PropertyInfo> properties) where T : class, new()
{
    while (!rs.EOF)
    {
        var obj = new T();
        foreach (var property in properties)
        {
            var val = rs.Fields[property.Name].Value;
            if (val != null)
            { 
                property.SetValue(obj, Convert.ChangeType(val, property.PropertyType));
            }
        }

        yield return obj;

        rs.MoveNext();
    }
}

Leave a Reply

Your email address will not be published. Required fields are marked *