3
\$\begingroup\$

Can I optimize the BuildEntity method more?

protected IEnumerable<GroupTitle> BuildProductGroup()
    {
        var productGroups = new List<GroupTitle>();
        using (var connection = new SqlConnection(_dbConnection))
        using (var command = new SqlCommand(_getProductGroups, connection))
        {
            connection.Open();
            command.CommandType = CommandType.StoredProcedure;
            using (var reader = command.ExecuteReader())
                while(reader.Read())
                {
                    var productGroup = new GroupTitle();
                    BuildEntity<GroupTitle>(reader, ref productGroup);
                    productGroups.Add(productGroup);
                }
        }

        return productGroups;
    }

    protected TEntity BuildEntity<TEntity>(IDataReader reader, TEntity model)
    {
        var type = model.GetType();
        var table = Enumerable.Range(0, reader.FieldCount).Select(reader.GetName).ToArray();

        foreach (var column in table)
        {
            var matchColumnToProperty = type.GetProperties().FirstOrDefault(property => String.Compare(property.Name, column, true) == 0);
            if (matchColumnToProperty != null && !reader.IsDBNull(reader.GetOrdinal(matchColumnToProperty.Name)))
                matchColumnToProperty.SetValue(model, reader.GetValue(reader.GetOrdinal(matchColumnToProperty.Name)), null);
        }

        return model;
    }
\$\endgroup\$
3
  • \$\begingroup\$ Is there a reason you need to roll your own ORM? There's a big chance that the solution you want is out there already. \$\endgroup\$ Commented Feb 18, 2016 at 15:15
  • \$\begingroup\$ I built something like this once, but rather than dynamically using reflection to search for a matching property every time, I used custom attributes on classes to map them to a table and on properties to match them to a column (decoupling property names from column names). When my program ran, it used reflection to build a mapping of classes/properties to tables/columns and cached it. Reading (or writing) to the database just required following the mapping. This also required substantially more code, of course, and this was an extension to an already existing very basic ORM. \$\endgroup\$ Commented Feb 18, 2016 at 15:54
  • \$\begingroup\$ Really more for learning. Otherwise I would simply use Entity Framework or Dapper. \$\endgroup\$ Commented Feb 18, 2016 at 18:02

2 Answers 2

1
\$\begingroup\$

You can cache the properties to improve performance. No need to retrieve them repeatedly within the body of that for loop.

Also, simplify the LINQ expression to gather all the matching properties, and then iterate over the result set in one fell swoop.

protected IEnumerable<GroupTitle> BuildProductGroup()
{
    var productGroups = new List<GroupTitle>();
    using (var connection = new SqlConnection(_dbConnection))
    using (var command = new SqlCommand(_getProductGroups, connection))
    {
        connection.Open();
        command.CommandType = CommandType.StoredProcedure;
        using (var reader = command.ExecuteReader())
        {
            while(reader.Read())
            {
                var productGroup = new GroupTitle();
                BuildEntity<GroupTitle>(reader, ref productGroup);
                productGroups.Add(productGroup);
            }
        }
    }

    return productGroups;
}

protected TEntity BuildEntity<TEntity>(IDataReader reader, TEntity model)
{
    var type = model.GetType();
    var table = Enumerable.Range(0, reader.FieldCount).Select(reader.GetName).ToArray();

    var properties = type.GetProperties();
    var matchedColumns = properties.Where(property => string.Compare(property.Name, 
                                                                     column, 
                                                                     true) == 0 
                                                      && !reader.IsDBNull(reader.GetOrdinal(matchColumnToProperty.Name)));

    foreach (var property in matchedColumns)
    {
        var value = reader.GetValue(reader.GetOrdinal(matchColumnToProperty.Name));
        matchColumnToProperty.SetValue(model, value, null);
    }

    return model;
}
\$\endgroup\$
1
  • \$\begingroup\$ Where did you get column, that isn't in scope? This code doesn't appear to work. \$\endgroup\$ Commented Mar 9, 2016 at 23:16
1
\$\begingroup\$

I ended up improving performance and tidying the code up a bit by doing the following:

    public IList<TEntity> List<TEntity>(string query, CommandType commandType, params SqlParameter[] parameters) where TEntity : class, new()
    {
        using (var connection = new SqlConnection(dbConnection))
        using (var command = new SqlCommand(query, connection))
        {
            connection.Open();
            command.CommandType = commandType;

            foreach (var parameter in parameters)
                command.Parameters.Add(parameter);

            return BuildEntity(command, new TEntity());
        }
    }

Then BuildEntity would be as follows:

    public List<TEntity> BuildEntity<TEntity>(SqlCommand command, TEntity entity) where TEntity : class, new()
    {
        var collection = new List<TEntity>();
        var properties = GetColumnDataFromProperty<TEntity>();
        using (var reader = command.ExecuteReader())
            while(reader.Read())
                collection.Add(MapEntity<TEntity>(reader, properties));

        return collection;
    }
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.