Generating variable names and getters from member expression

Question

I have a JSON configuration file where the user is allowed to use inside strings certain placeholders {Type.Name} that later will be interpolated with the actual values (for connection strings, titles, texts, summaries etc.).

For example in report settings he might want to use the TestCase.Severityvalue

"Title": "{TestCase.Severity}",

Each variable consists of the name of the declaring type and the name of the property.

I did not want to hardcode those properties (there are a lot more of them) so I build a VariableBuilder. It uses a simple member and getter expression to generate names and resolve values. The variables are stored in a dictionary keyed with the declaring type and a hash-set that keeps a type variables.

internal class VariableBuilder
{
    private readonly IDictionary<Type, HashSet<INameable>> _variables = new Dictionary<Type, HashSet<INameable>>();

    public void AddVariable<T>(Expression<Func<T, object>> expression)
    {
        if (expression.Body is MemberExpression memberExpression)
        {
            var variableName = $"{memberExpression.Member.DeclaringType.Name}.{memberExpression.Member.Name}";

            var instance = Expression.Parameter(typeof(T), "obj");
            var property = Expression.Property(instance, memberExpression.Member.Name);
            var getValue = Expression.Lambda<Func<T, object>>(property, instance).Compile();

            if (_variables.TryGetValue(typeof(T), out var variables))
            {
                if (!variables.Add(Variable<T>.Create(variableName, getValue)))
                {
                    throw new ArgumentException($"Variable \"{variableName}\" has already been added.");
                }
            }
            else
            {
                _variables.Add(typeof(T), new HashSet<INameable> { Variable<T>.Create(variableName, getValue) });
            }
        }
        else
        {
            throw new ArgumentException("Expression must be a member expression.");
        }
    }

    public IEnumerable<KeyValuePair<string, object>> ResolveVariables<T>(T obj)
    {
        if (_variables.TryGetValue(typeof(T), out var variables))
        {
            foreach (var variable in variables.Cast<Variable<T>>())
            {
                yield return new KeyValuePair<string, object>(variable.Name, variable.GetValue(obj));
            }
        }
    }
}

To prevent name conflicts I use the INameable interface that requires the Variable<T> to also implement the IEquatable interface so that the hash-set can do its job right.

internal interface INameable : IEquatable<INameable>
{
    string Name { get; }
}

The actual variable name and the getter func are stored by the Variable<T>.

internal class Variable<T> : INameable
{
    private Variable(string name, Func<T, object> getValue)
    {
        Name = name;
        GetValue = getValue;
    }

    public string Name { get; }

    public Func<T, object> GetValue { get; }

    public static INameable Create(string name, Func<T, object> getValue)
    {
        return new Variable<T>(name, getValue);
    }

    public bool Equals(INameable nameable)
    {
        return Name.Equals(nameable.Name);
    }

    public override int GetHashCode() => Name.GetHashCode();
}

Here are a few examples of how I use it. When a test is run and a report should be generated I pass the actual test objects to the variable-builder that resolves the final values and later during report generation another utility (VariableResolver) can interpolate them into strings.

var variableReader = new VariableBuilder();
variableReader.AddVariable<TestFile>(x => x.Name);
//variableReader.AddVariable<TestFile>(x => x.Name); // throws because of duplicte key
variableReader.AddVariable<TestUnit>(x => x.Filter);

var testFile = new TestFile { Name = "foo" };
var testUnit = new TestUnit { Filter = "bar" };

var testFileVariables = variableReader.ResolveVariables(testFile);
var testUnitVariables = variableReader.ResolveVariables(testUnit);

The two types used in the above example (those are not some test objects, the names are real but the actual objects just have a few more properties):

internal class TestFile
{
    public string Name { get; set; }
}

internal class TestUnit
{
    public string Filter { get; set; }
}

If someone is curious how such an configuration might look like, here's an example. — t3chb0t
– t3chb0t, Commented Jun 13, 2017 at 4:35

Denis · Accepted Answer · 2017-06-12 21:28:02Z

Unnecessary expression trees

Expression.Compile() is quite slow, on top of that it requires you to create some extra variables in order to be able to create the expression tree which you later compile to a delegate. Why not create the delegate right away?

I'm still getting used to the C# 7 features but this should be a proper declaration of a local function:

object GetValue(T t) => t.GetType().GetProperty(memberExpression.Member.Name).GetValue(t);

Accessing the type name shortly

You can replace memberExpression.Member.DeclaringType.Name with typeof(T).Name as the declaring type will always be the generic argument in your expression.

Reducing the nesting

You can invert the first condition in your AddVariable<T> to reduce the nesting and shorten the method:

if (!(expression.Body is MemberExpression memberExpression))
{
    throw new ArgumentException("Expression must be a member expression.");
}
/...

LINQ instead of `foreach`

You can transform your ResolveVariables method into a LINQ version, which doesn't use an iterator which looks better to me:

public IEnumerable<KeyValuePair<string, object>> ResolveVariables<T>(T obj)
{
    if (_variables.TryGetValue(typeof(T), out var variables))
    {
        return variables.Cast<Variable<T>>()
            .Select(t => new KeyValuePair<string, object>(t.Name, t.GetValue(obj)));
    }
    return Enumerable.Empty<KeyValuePair<string, object>>();
}

Quotation marks

It looks clearer using a verbatim string when you want to add quotations:

throw new ArgumentException($@"Variable ""{variableName}"" has already been added.");

vs

throw new ArgumentException($"Variable \"{variableName}\" has already been added.");

Is it really necessary to throw an exception here tho? Why not just let it slide..

Poor equality check

I'm not sure if that simplified version of your class but if that's how it looks, at the very least you should do some null checks, as both Name and nameable can be null and this will throw an exception which violates the guidelines for overriding Equals(). Something along those lines should be more stable:

public override int GetHashCode()
{
    unchecked
    {
        return ((Name != null ? Name.GetHashCode() : 0) * 397) ^
                (GetValue != null ? GetValue.GetHashCode() : 0);
    }
}

protected bool Equals(Variable<T> other)
{
    return string.Equals(Name, other.Name);
}

public bool Equals(INameable other)
{
    if (ReferenceEquals(null, other))
    {
        return false;
    }
    if (ReferenceEquals(this, other))
    {
        return true;
    }
    return other.GetType() == GetType() && Equals((Variable<T>) other);
}

You can change it a bit for example returning base.GetHashCode() instead of 0 if some value is null. And for this simple class you can even get rid of the protected Equals method, I prefer having it tho, because if the logic gets complicated it is separated and the public method is easier to read.

Just a few comments. I use the Compile only once per property when the application loads so this time it's not a big performance consideration. Without expressions I won't be able to get the values later. I don't know how to create the Fun<> to get those value otherwise (correct me if I missed something in your answers). — t3chb0t
– t3chb0t, Commented Jun 13, 2017 at 4:11
The equality check is correct ;-) I'm interested only in the name. This and the exception is for me so that I don't create variables twice. The value is resolved later when I know the actual instance that will provide a value. I don't know them when the program starts. They are known after tests are run and just before reports are generated. There can be several tests with different conditions, messages, severities and expressions, see example. — t3chb0t
– t3chb0t, Commented Jun 13, 2017 at 4:13
this is how I define them now. Everything is hardcoded and is very inconvenient to extend which is why I want to create the Funcs first and later just pass the instances and generate the names when the program start and resolve values later. — t3chb0t
– t3chb0t, Commented Jun 13, 2017 at 4:28
One line after the comment about the expression tree, I included an example solution with a local function. If you really feel like throwing exceptions is the better option in the Equals method fine but to me it seems inconsistent as other types wont exhibit the same behavior on this same method. It's like returning 1, 2, 3 from CompareTo when implementing IComparable<> because you decided -1, 0, 1 didn't fit your solution. You can do that but it doesn't sound right. — Denis
– Denis, Commented Jun 13, 2017 at 10:51
Well, the Equals method does not throw, its only job is to tell whether two names are equal ;-) because I don't want to have any variable twice and that's why I use a hash-set - it understands IEquatable and its Add method returns true/false after asking the interface what it should do with the new item. It's for me and the exception is going to be thrown if I made a mistake so the application won't start. — t3chb0t
– t3chb0t, Commented Jun 13, 2017 at 13:37

Stack Exchange Network

Generating variable names and getters from member expression

1 Answer 1

Unnecessary expression trees

Accessing the type name shortly

Reducing the nesting

LINQ instead of `foreach`

Poor equality check

You must log in to answer this question.

Hot Network Questions

Generating variable names and getters from member expression

1 Answer 1

Unnecessary expression trees

Accessing the type name shortly

Reducing the nesting

LINQ instead of foreach

Poor equality check

You must log in to answer this question.

Related

Hot Network Questions

LINQ instead of `foreach`