Using a lambda expression to avoid using a "magic string" to specify a property

Question

I am writing a service to take a collection of objects of a particular type and output its primitive, string, and DateTime types to a string in CSV Format. I have both of the below statements working. I find the the lambda based version to be much cleaner.

Magic String Version

string csv = new ToCsvService<DateTime>(objs)
    .Exclude("Minute")
    .ChangeName("Millisecond", "Milli")
    .Format("Date", "d")
    .ToCsv();

vs. Lambda Version

string csv = new ToCsvService<DateTime>(objs)
    .Exclude(p => p.Minute)
    .ChangeName(p => p.Millisecond, "Milli")
    .Format(p => p.Date, "d")
    .ToCsv();

Per Jon Skeet's recommendation all of the lambda methods share a similar method signature

public IToCsvService<T> Exclude<TResult>(
        Expression<Func<T, TResult>> expression)

I then pass the expression.Body to FindMemberExpression. I've adapted code from the FindMemberExpression method of ExpressionProcessor.cs from the nhlambdaextensions project. My very similar version of FindMemberExpression is below:

private string FindMemberExpression(Expression expression)
{
    if (expression is MemberExpression)
    {
        MemberExpression memberExpression = (MemberExpression)expression;

        if (memberExpression.Expression.NodeType == ExpressionType.MemberAccess
            || memberExpression.Expression.NodeType == ExpressionType.Call)
        {
            if (memberExpression.Member.DeclaringType.IsGenericType
                && memberExpression.Member.DeclaringType
                .GetGenericTypeDefinition().Equals(typeof(Nullable<>)))
            {
                if ("Value".Equals(memberExpression.Member.Name))
                {
                    return FindMemberExpression(memberExpression.Expression);
                }

                return String.Format("{0}.{1}",
                    FindMemberExpression(memberExpression.Expression),
                    memberExpression.Member.Name);
            }
        }
        else
        {
            return memberExpression.Member.Name;
        }
    }

    throw new Exception("Could not determine member from "
        + expression.ToString());
}

I am testing for enough cases in FindMemberExpression? Is what I am doing overkill given my use case?

Answer 1

EDIT: The core to making this simpler is to change the signature of your methods to be generic in the result type too:

public IToCsvService<TSource> Exclude<TResult>(
    Expression<Func<TSource, TResult>> expression)

That way you won't end up with a conversion expression because no conversion will be necessary. For example, p => p.Minute will end up as an Expression<Func<DateTime, int>> automatically due to type inference.

---

It looks like overkill to me, given that at the moment all you need is a property - at least, that's all that your sample shows.

Why not start off just recognising a property, and expand it later if you need to?

EDIT: Here's a short but complete example which doesn't show any conversions:

using System;
using System.Linq.Expressions;

class Test
{
    static void Main()
    {
        Expression<Func<DateTime, int>> dt = p => p.Minute;
        Console.WriteLine(dt);
    }
}

If you change the expression type to Expression<Func<DateTime, long>> however, it does show the Convert(...) bit. I suspect you need to change the signatures of your Exclude (etc) methods.

Answer 2

Do you have any plans on make it more flexible, or is this all that it needs to do?

Ideally you should have the simplest code that will do what you need done, so that you can decrease the number of things that can go wrong.

If you are doing this as a proof of concept, and know that later you will need lambda expressions, then it makes sense to keep them there, but, if this is the final product then the former one is easier to read, and less likely to be a cause of confusion if someone else needs to make changes to the code.