Function that builds dictionary based on lambda params

I've written a method in c# that allows me to do create a dictionary from a passed in object and N lambda expressions that reference that objects properties and methods. It's working the way I want it and seems to perform well, but I'm looking for criticism and seeking to improve the code.

Here's an example usage:

class Person
{
    public string Name { get; set; }
    public int Age { get; set; }        

    public Dictionary MakeDictionary()
    {
        return DictionaryThing.Data(this, x => x.Name, x => x.Age);             

        /* example return dictionary:

          Name: "Ronnie"
          Age: 29

        */
    }
}

And here's the code for DictionaryThing:

static class DictionaryThing 
{

    static ConcurrentDictionary expCache = new ConcurrentDictionary();

    public static Dictionary Data(T obj, params Expression>[] expressions)
    {
        var dict = new Dictionary();

        foreach (var exp in expressions)
        {
            string name = null;
            var body = exp.Body;    

            var unaryExp = body as UnaryExpression;
            if (unaryExp != null)
                body = unaryExp.Operand;

            var memberExp = body as MemberExpression;
            if (memberExp != null)
                name = memberExp.Member.Name;

            var methodCallExp = body as MethodCallExpression;
            if (methodCallExp != null)
                name = methodCallExp.Method.Name;

            if (name == null)
                throw new InvalidExpressionException(
                    string.Format("The expression '{0}' is invalid. You must supply an expression that references a property or a function of the type '{1}'.",
                        exp.Body, typeof(T)));

            var key = typeof(T).FullName + "." + name;          
            var func = (Func)expCache.GetOrAdd(key, k => ((LambdaExpression)exp).Compile());

            dict[name] = func(obj);
        }

        return dict;
    }

}

My concern

Ronnie,

You have a good starting point for the problem you are trying to solve.

To answer your concerns:

-
Is there a better way to work with the expressions that are passed in.

I think what you have done is fine. You are using Expression to let user specify methods/properties in type safe way. The only thing I would change is to use Expression>[] instead of Expression>[] as dynamic will invoke the compiler at run time.

-
Can the expressions parameter be better constrained by a more appropriate type?

I don't think you can specify additional constraints at compile time but you can definitely add code to validate the expression at runtime.

-
Is there a better way to derive the cache key?

I think your cache key is fine. If you want to be more conservative, you can include Type.AssemblyQualifiedName. I would worry about it only if I was releasing to general public and not if I am using this class as part of a team.

With that said, here's the full re-factored code along with my notes. I hope it helps.

``
void Main()
{
var ronnie = new Person();
ronnie.Name = "Ronnie";
ronnie.Address = "Greensboro, NC";

var dictionary = ObjectToDictionaryConverter.GetDictionary(ronnie, x => x.Name, x => x.Address);

// In following example, we are trying to pass in an expression which is not valid for our object.
// Running this code will throw an appropriate exception like
// Expression value(UserQuery+<>c__DisplayClass0).anotherDictionary.Keys is invalid.
// Expression Property/Member Type System.Collections.Generic.Dictionary2[[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089],[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]]
//, expecting Type: UserQuery+Person

//var anotherDictionary = new Dictionary();
//var dictionary = ObjectToDictionaryConverter.GetDictionary(ronnie, x => anotherDictionary.Keys);

//I don't think you can enforce this constraint at compile time (I am not sure about it, I would love to be proved wrong.)

Console.WriteLine(dictionary["Name"]); //Prints Ronnie
Console.WriteLine(dictionary["Address"]); //Prints Greensboro, NC

}

public class Person
{
public string Name { get; set; }

public string Address { get; set; }
}

// Why would you name a class like 'DictionaryThing'? It doesn't explain anything about what the class does.
// Let's give it a more descriptive 'ObjectToDictionaryConverter' name. You may prefer some other name but
// make sure that you put same thought in to naming your class as the code which goes inside the class.

// Next, the original code was doing two things
// 1. Inspecting the expression to derive a key
// 2. Create the dictionary from the input object (while also catching the expression details)

// Following SRP, let's separate it out in two classes so that each class is responsible for doing one thing.
// This also means that you can use the Expression inspection code somewhere else if you choose to.
public static class ObjectToDictionaryConverter
{
//Always spell out the variable name in its full. (expCache -> expressionCache)
static ConcurrentDictionary expressionCache = new ConcurrentDictionary();

public static Dictionary GetDictionary(T obj, params Expression>[] expressions)
{
//Always validate your arguments.
if(ReferenceEquals(obj, null))
throw new ArgumentNullException("obj");

if(expressions.Length == 0)
throw new ArgumentException("You must specify at least one expression.");

foreach (var expression in expressions)
{
if(expression == null)
throw new ArgumentException("You can not specify NULL expression.");
}

var result = new Dictionary();

foreach (var expression in expressions)
{
//The purpose of ExpressionDetail is to inspect our expression
var expressionDetail = ExpressionDetail.Create(expression);

//A lambda expression can be a valid expression referring to a property or function.
//But for our need, we will need to compile this expression to a delegate and run on object of Type T, let's make
//sure that expression refers to the correct type.

//IMPORTANT: We should not put this check in ExpressionDetail class. It is not
//the responsibility of ExpressionDetail to enforce this type constraint.
//This type constraint is only needed for ObjectToDictionaryConverter class.
if(expressionDetail.DeclaringType != typeof(T))
throw new InvalidExpressionException("Expression " + expression.Body + " is invalid. Expression Property/Member Type " + expressionDetail.DeclaringType.FullName + ", expecting Type: " + typeof(T).FullName);

StackExchange Code Review Q#33152, answer score: 5