Populating dropdown lists, using SQL vs. using expression trees and reflection

Question

I'm working with a large enterprise SQL Server database with dozens of tables that are used mainly as lookups to populate dropdown lists, etc. These tables all follow the convention of having (table)Name as the string value of the item. For example the State table has a StateName column with values like California, Florida, etc.

There is an old method used to load data for a dropdown that uses raw SQL, it looks like this:

public async Task<IEnumerable<string>> GetLookupOptions(string table)
{
    List<string> values = new List<string>();

    using (var command = _context.Database.GetDbConnection().CreateCommand())
    {
        command.CommandText = $"SELECT {table}Name FROM {table} WHERE IsApproved = 1";

        _context.Database.OpenConnection();

        using (var result = await command.ExecuteReaderAsync())
        {
            do
            {
                while (result.Read())
                {
                    values.Add(result.GetString(0));
                }
            } while (result.NextResult());
        }
    }

    return values;
}

This isn't testable using InMemoryDatabase and only works against an actual database connection. I have rewritten it using Reflection and Expressions to query the DbContext.DbSet<T> based on the table name provided, like this:

public IEnumerable<string> GetLookupOptions(string table)
{
    // Get the Type for State and then replace its name with whatever entity this is querying.
    // This is hacky but it's used so the AssemblyQualifiedName will always have correct
    // version info.
    Type t = typeof(State);
    t = Type.GetType(t.AssemblyQualifiedName.Replace("State", table));

    // Get lambda used to filter <table> where IsApproved is true.
    object whereLamda = this.GetType()
        .GetMethod(nameof(CreateWhereExpression), BindingFlags.NonPublic | BindingFlags.Instance)
        .MakeGenericMethod(t)
        .Invoke(this, new object[0]);

    // Get lambda used to select <table>Name from <table>.
    object selectLamda = this.GetType()
        .GetMethod(nameof(CreateSelectExpression), BindingFlags.NonPublic | BindingFlags.Instance)
        .MakeGenericMethod(t)
        .Invoke(this, new object[] { table });

    // Get the DbSet<T> for the <table>.
    object set = _context.GetType()
        .GetMethod("Set")
        .MakeGenericMethod(t)
        .Invoke(_context, new object [0]);

    IEnumerable<MethodInfo> whereMethods = typeof(Enumerable)
        .GetMethods().Where(m => m.Name == nameof(Enumerable.Where));

    // Apply Where() method to DbSet.
    object filteredApproved = whereMethods
        .ElementAt(0)
        .MakeGenericMethod(t)
        .Invoke(set, new object[] { set, whereLamda });

    IEnumerable<MethodInfo> selectMethods = typeof(Enumerable)
        .GetMethods().Where(m => m.Name == nameof(Enumerable.Select));

    // Apply Select() method to filtered query.
    object r = selectMethods
        .ElementAt(0)
        .MakeGenericMethod(t, typeof(string))
        .Invoke(filteredApproved, new object[] { filteredApproved, selectLamda });

    return r as IEnumerable<string>;
}

private Func<T, string> CreateSelectExpression<T>(string tableName)
{
    var param = Expression.Parameter(typeof(T));
    var expr = Expression.Property(param, $"{tableName}Name");

    return Expression.Lambda<Func<T, string>>(expr, param).Compile();
}

private Func<T, bool> CreateWhereExpression<T>()
{
    var param = Expression.Parameter(typeof(T));
    var expr = Expression.Equal(
        Expression.Property(param, "IsApproved"),
        Expression.Constant(true));

    return Expression.Lambda<Func<T, bool>>(expr, param).Compile();
}

My concerns are:

• Getting the correct Where and Select methods from Reflection, I don't like relying on ElementAt(). Both methods have overloads with 2 parameters.
• Any and all other problems. Have I created a monster?

Answer 1

I'm going to start off by saying I didn't test this code. There might be some issues but I didn't setup EF and an In Memory Provider.

First I would load all the types right away and cache them and do some filtering to make sure they have the two properties we care about.

You didn't save what your class name is so I'm going with DropDownLookUp

public class DropDownLookUp
{

    private DbContext _context;

    private readonly static IDictionary<string, Type> Mappings;
    private readonly static MethodInfo WhereMethod;
    private readonly static MethodInfo SelectMethod;

    static DropDownLookUp()
    {
        // Load up all the types and cache them making sure they have the properties we need
        Mappings = typeof(State).Assembly.GetLoadableTypes()
            .Where(x => x.GetProperty("IsApproved")?.PropertyType == typeof(bool) &&
                   x.GetProperty(x.Name + "Name")?.PropertyType == typeof(string))
                   .ToDictionary(x => x.Name, x => x);

        Func<IQueryable<object>, Expression<Func<object, bool>>, IQueryable<object>> whereMethod = Queryable.Where;
        WhereMethod = whereMethod.Method.GetGenericMethodDefinition();

        Func<IQueryable<object>, Expression<Func<object, object>>, IQueryable<object>> selectMethod = Queryable.Select;
        SelectMethod = selectMethod.Method.GetGenericMethodDefinition();
    }

We do a one time load of the LoadableTypes and have a type safe way to grab the method infos for the Queryable Select and Where clauses.

Type load I got from StackOverFlow it's like this

public static class AssemblyExtensions
{
    public static IEnumerable<Type> GetLoadableTypes(this Assembly assembly)
    {
        if (assembly == null)
        {
            throw new ArgumentNullException("assembly");
        }
        try
        {
            return assembly.GetTypes().Where(x => x.IsPublic);
        }
        catch (ReflectionTypeLoadException e)
        {

            return e.Types.Where(t => t != null && t.IsPublic);
        }
    }
}

Now we don't want to drop down into IEnumerable until we have to. We want to stay in ExpressionTrees and build up an IQueryable.

private Expression CreateSelectExpression(Expression source)
{
    var param = Expression.Parameter(source.Type);
    var project = Expression.Property(param, $"{source.Type.Name}Name");
    var selector = Expression.Lambda(typeof(Func<,>).MakeGenericType(source.Type, typeof(string)), project, param);
    return Expression.Call(SelectMethod.MakeGenericMethod(source.Type, typeof(string)), source, selector);
}

private Expression CreateWhereExpression(Expression source)
{
    var param = Expression.Parameter(source.Type);
    var filter = Expression.Equal(
        Expression.Property(param, "IsApproved"),
        Expression.Constant(true));

    var whereClause = Expression.Lambda(typeof(Func<,>).MakeGenericType(source.Type, typeof(bool)), filter, param);
    return Expression.Call(WhereMethod.MakeGenericMethod(source.Type), source, whereClause);
}

These methods now call the Queryable.Select and Queryable.Where. If you want you could cache these methods per Type but I find building Expressions isn't that taxing, where compiling them is, but it's worth testing to see if you want to add some cache to these methods.

For the main method we can just check the Mappings and if not found either return empty enumerable or throw. We can still use the reflection on Set. If you really wanted you could build an expression tree to give you back the IQueryable when passing in a type. I would base that on if you have performance issues or not.

public async Task<IEnumerable<string>> GetLookupOptions(string table)
{
    Type type;
    if (!Mappings.TryGetValue(table, out type))
    {
        // Or you can throw your choice
        return Enumerable.Empty<string>();
    }

    // Get the DbSet<T> for the <table>.
    IQueryable set = (IQueryable)_context.GetType()
        .GetMethod("Set")
        .MakeGenericMethod(type)
        .Invoke(_context, new object[0]);

    var query = set.Provider.CreateQuery<string>(CreateSelectExpression(CreateWhereExpression(set.Expression)));
    return await query.ToListAsync().ConfigureAwait(false);
}

We can use the IQueryable Provider to create a new Query based on the expressions we build - which should execute the where and select up on the server and we can now use the ToListAsync to make the method async again.