4
\$\begingroup\$

Over the weekend I decided to start work on my own version of FastMember. I began with my take on an implementation of TypeAccessor so that I could work with the properties of any type, create an instance of the type, and one or two other little tricks. I've reached a point where I'm out of ideas on how to improve the design of the class further so now I'm looking for feedback.

Usage:

public class Point2D {
    public double X { get; set; }
    public double Y { get; set; }
}

var pointA = new Point2D { X = 9000.01, Y = 0.0 };
var accessor = TypeAccessor.Create(pointA);
var pointB = new Point2D(); // pointB.X should be 0.0

accessor.CloneProperties(pointA, pointB) // pointB.X should now be 9000.01
accessor[pointA, "X"] = 0.0; // sets pointA.X to 0.0
Console.WriteLine(accessor[pointA, "X"]) // prints pointA.X, should be 0.0
accessor.Default(ref pointA); // sets pointA's properties to default the accessor's default values
Console.WriteLine(accessor[pointA, "X"]); // prints pointA.X, should be 9000.01

Notes:

One of the design goals was to ensure that everything always works as you might expect it to based on the current context. If the Type you send to TypeAccessor has a property that is read-only then a method like CloneProperties will simply ignore that property because it's not cloneable. GetProperties returns an actual instance of Dictionary<string, object> because it makes no sense to let the method yield when:

  1. Some nasty behaviors become possible because reference types could be nulled during any yield
  2. Two other options exist for accessing properties one at a time

As far as I know, every single "expensive" operation that can be lifted into the constructor has been (which was very important to me for performance). I can't say I'm pleased with the amount of code there but it really doesn't belong anywhere else. I suppose it could be split up into two or three private methods but I'm not sure that would be any better...

Dependencies:

There are three method calls in this class that are not part of .NET: GetGetAccessor, GetSetAccessor, and HasDefaultConstructor. Links to their implementation is provided below for reference.

PropertyInfoExtensions.cs

TypeExtensions.cs

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.Serialization;

public interface ITypeAccessor<T>
{
    object this[T instance, string propertyName] { get; set; }
    IEnumerable<string> CloneableProperties { get; }
    IDictionary<string, Func<T, object>> GetterCache { get; }
    IDictionary<string, Action<T, object>> SetterCache { get; }

    void CloneProperties(T source, T target);
    void Default(ref T instance);
    object GetProperty(T instance, string propertyName);
    IDictionary<string, object> GetProperties(T instance, IEnumerable<string> properties);
    T New();
    void SetProperty(T instance, string propertyName, object value);
    void SetProperties(T instance, IEnumerable<KeyValuePair<string, object>> properties);
}

/// <summary>
/// Provides cached access to a <see cref="T"/>'s property getters and setters.
/// </summary>
public class TypeAccessor<T> : ITypeAccessor<T>
{
    private readonly Func<T, T> m_applyDefaultValues;
    private readonly ReadOnlyCollection<string> m_cloneableProperties;
    private readonly Func<T> m_constructType;
    private readonly ReadOnlyDictionary<string, Func<T, object>> m_getterCache;
    private readonly ReadOnlyDictionary<string, Action<T, object>> m_setterCache;

    /// <summary>
    /// Uses the <see cref="TypeAccessor{_}"/> to get or set the value of a property by name.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to get or set a property on.</param>
    /// <param name="propertyName">The name of the property to affect on the instance of <see cref="T"/>.</param>
    /// <returns></returns>
    public object this[T instance, string propertyName] {
        get {
            return GetProperty(instance, propertyName);
        }
        set {
            SetProperty(instance, propertyName, value);
        }
    }
    /// <summary>
    /// Returns a read-only cache of property names that belong to <see cref="T"/> and have both a getter and a setter.
    /// </summary>
    public IEnumerable<string> CloneableProperties {
        get {
            return m_cloneableProperties;
        }
    }
    /// <summary>
    /// Returns a read-only cache of property getters that belong to <see cref="T"/>.
    /// </summary>
    public IDictionary<string, Func<T, object>> GetterCache {
        get {
            return m_getterCache;
        }
    }
    /// <summary>
    /// Returns a read-only cache of property setters that belong to <see cref="T"/>.
    /// </summary>
    public IDictionary<string, Action<T, object>> SetterCache {
        get {
            return m_setterCache;
        }
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="TypeAccessor{_}"/> class.
    /// </summary>
    /// <param name="defaultValue">The instance of <see cref="T"/> that will be used as this <see cref="TypeAccessor"/>'s default value.</param>
    /// <param name="includeNonPublic">Indicates whether non-public properties should be included.</param>
    public TypeAccessor(T defaultValue, bool includeNonPublic = false) {
        var properties = typeof(T).GetProperties(BindingFlags.Instance | (includeNonPublic ? BindingFlags.NonPublic : BindingFlags.Default) | BindingFlags.Public);

        m_getterCache = new ReadOnlyDictionary<string, Func<T, object>>(properties
            .Select(propertyInfo => new {
                PropertyName = propertyInfo.Name,
                PropertyGetAccessor = propertyInfo.GetGetAccessor<T>(includeNonPublic)
            })
            .Where(a => a.PropertyGetAccessor != null)
            .ToDictionary(
                k => k.PropertyName,
                v => v.PropertyGetAccessor
            ));
        m_setterCache = new ReadOnlyDictionary<string, Action<T, object>>(properties
            .Select(propertyInfo => new {
                PropertyName = propertyInfo.Name,
                PropertySetAccessor = propertyInfo.GetSetAccessor<T>(includeNonPublic)
            })
            .Where(a => a.PropertySetAccessor != null)
            .ToDictionary(
                k => k.PropertyName,
                v => v.PropertySetAccessor
            ));
        m_cloneableProperties = Array.AsReadOnly(GetterCache.Keys.Intersect(SetterCache.Keys).ToArray());

        if (typeof(T).IsValueType) {
            m_applyDefaultValues = (T instance) => { return defaultValue; };
            m_constructType = () => { return defaultValue; };
        }
        else if (defaultValue != null) {
            var defaultConstructor = GetDefaultConstructor();
            var propertyValues = GetProperties(defaultValue, CloneableProperties).ToArray();

            m_applyDefaultValues = (T instance) => { SetProperties(instance, propertyValues); return instance; };
            m_constructType = () => { return m_applyDefaultValues(defaultConstructor()); };
        }
        else {
            m_applyDefaultValues = (T instance) => { return default(T); };
            m_constructType = () => { return default(T); };
        }
    }

    /// <summary>
    /// Retrieves all property values from a source <see cref="T"/> and applies them to a target <see cref="T"/>.
    /// </summary>
    /// <param name="source">The source instance of <see cref="T"/> to retrieve property values from.</param>
    /// <param name="target">The target instance of <see cref="T"/> to apply property values to.</param>
    public void CloneProperties(T source, T target) {
        SetProperties(target, GetProperties(source, CloneableProperties));
    }
    /// <summary>
    /// Sets all propertes on the specified instance of <see cref="T"/> to this <see cref="TypeAccessor{_}"/>'s default values.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to get a property from.</param>
    public void Default(ref T instance) {
        instance = m_applyDefaultValues(instance);
    }
    /// <summary>
    /// Uses the <see cref="TypeAccessor{_}"/> and specified <see cref="IEnumerable{_}"/> to get all property values.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to get properties from.</param>
    public IDictionary<string, object> GetProperties(T instance) {
        return GetterCache.Keys.ToDictionary(
            propertyName => propertyName,
            propertyName => GetProperty(instance, propertyName)
        );
    }
    /// <summary>
    /// Uses the <see cref="TypeAccessor{_}"/> and specified <see cref="IEnumerable{_}"/> to get property values by name.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to get properties from.</param>
    /// <param name="properties">The list of property names that values will be retrieved from the instance of <see cref="T"/>.</param>
    public IDictionary<string, object> GetProperties(T instance, IEnumerable<string> properties) {
        if (properties != null) {
            return properties.ToDictionary(
                propertyName => propertyName,
                propertyName => GetProperty(instance, propertyName)
            );
        }
        else {
            return new Dictionary<string, object>();
        }
    }
    /// <summary>
    /// Uses the <see cref="TypeAccessor{_}"/> to get the value of a property by name.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to get a property from.</param>
    /// <param name="propertyName">The name of the property to retrieve from the instance of <see cref="T"/>.</param>
    public object GetProperty(T instance, string propertyName) {
        Func<T, object> getter;

        if (GetterCache.TryGetValue(propertyName, out getter)) {
            return getter(instance);
        }
        else {
            throw new KeyNotFoundException(message: string.Format("a property getter with the name does not {0} exist on {1}", propertyName, typeof(T).FullName));
        }
    }
    /// <summary>
    /// Creates a new <see cref="T"/> using this <see cref="TypeAccessor{_}"/>'s default type constructor.
    /// </summary>
    public T New() {
        return m_constructType();
    }
    /// <summary>
    /// Uses the <see cref="TypeAccessor{_}"/> and specified <see cref="IEnumerable{KeyValuePair{_,_}}"/> to set property values by name.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to set property values for.</param>
    /// <param name="properties">The dictionary of property names and values that will be applied to the instance of <see cref="T"/>.</param>
    public void SetProperties(T instance, IEnumerable<KeyValuePair<string, object>> properties) {
        if (properties != null) {
            foreach (var property in properties) {
                SetProperty(instance, property.Key, property.Value);
            }
        }
    }
    /// <summary>
    /// Uses the <see cref="TypeAccessor{_}"/> to set the value of a property by name.
    /// </summary>
    /// <param name="instance">The instance of <see cref="T"/> to set a property value for.</param>
    /// <param name="propertyName">The name of the property to alter on the instance of <see cref="T"/>.</param>
    /// <param name="value">The value that will be applied to the property.</param>
    public void SetProperty(T instance, string propertyName, object value) {
        Action<T, object> setter;

        if (SetterCache.TryGetValue(propertyName, out setter)) {
            setter(instance, value);
        }
        else {
            throw new KeyNotFoundException(message: string.Format("a property setter with the name does not {0} exist on {1}", propertyName, typeof(T).FullName));
        }
    }

    /// <summary>
    /// Generates a <see cref="Func{_}"/> delegate to the default constructor of <see cref="T"/>.
    /// </summary>
    public static Func<T> GetDefaultConstructor() {
        var type = typeof(T);

        if (type == typeof(string)) {
            return Expression.Lambda<Func<T>>(Expression.TypeAs(Expression.Constant(null), typeof(string))).Compile();
        }
        else if (type.HasDefaultConstructor()) {
            return Expression.Lambda<Func<T>>(Expression.New(type)).Compile();
        }
        else {
            return () => (T)FormatterServices.GetUninitializedObject(type);
        }
    }
}
/// <summary>
/// Provides cached access to a <see cref="T"/>'s property getters and setters.
/// </summary>
public static class TypeAccessor
{
    /// <summary>
    /// Creates a new instance of the <see cref="ITypeAccessor{_}"/> interface using the specified <see cref="T"/>.
    /// </summary>
    /// <param name="defaultValue">The instance of <see cref="T"/> that will be used as this <see cref="ITypeAccessor{_}"/>'s default value.</param>
    /// <param name="includeNonPublic">Indicates whether non-public properties should be included.</param>
    public static ITypeAccessor<T> Create<T>(T defaultValue, bool includeNonPublic = false) {
        return new TypeAccessor<T>(defaultValue, includeNonPublic);
    }
}

Edit: Per request, I've added an interface at top to act as a sort of outline for all the class's desired functionality.

\$\endgroup\$
5
  • \$\begingroup\$ Can you please give an extended list of the desired functionalities ? I'd like to offer an alternative solution because this looks overly-engineered to me, but I'm afraid I might miss some of your functionality if I'm not aware of that. \$\endgroup\$
    – Denis
    Dec 29, 2016 at 17:42
  • \$\begingroup\$ @denis I have updated the example to demonstrate all of the desired functionality. The main idea is that I want to be able to manipulate the properties of virtually any object and define "default constructors" at run-time. In order to save expensive reflection calls during every property access the TypeAccessor class performs all necessary reflection in one shot and builds delegates for property manipulation and type construction. \$\endgroup\$ Dec 29, 2016 at 17:54
  • \$\begingroup\$ Thank you for that, but the examples are not functioning. There is a misspelled variable name point. \$\endgroup\$
    – Denis
    Dec 29, 2016 at 17:58
  • \$\begingroup\$ Can you clarify what is a default constructor ? I assume it's not the empty one ? \$\endgroup\$
    – Denis
    Dec 29, 2016 at 18:30
  • \$\begingroup\$ @denis The instance that is provided to TypeAccessor is used as a template for construction. All cloneable properties become the default values for any T constructed by the TypeAccessor. \$\endgroup\$ Dec 29, 2016 at 18:37

3 Answers 3

3
\$\begingroup\$

Looking at your code :

First I like to point out that it's well documented and to be honest I didn't even need to read the comments, the methods were speaking well enough to understand what's they're purpose, good job !

Compile-time typesafety

Your implementation is not compile-time safe which is bad, you can always miss-spell a variable's name which will result in an exception.

Indexer ?

Your indexer feels weird and doesn't quite fits the overall design, usually it suggests that you edit some internal collection, but that's not the purpose of yours. It accepts some random instance and sets some values to it, which is fine, but more suitable for this situation would be a different method. I believe you made it so that you can quickly change values of instances but that's not the way

Code style

You can shorten quite some stuff in your code with the help of C# 6

  • Get only properties.
public ReadOnlyCollection<string> CloneableProperties
{
    get
    {
        return m_cloneableProperties;
    }
}

public ReadOnlyDictionary<string, Func<T, object>> GetterCache
{
    get
    {
        return m_getterCache;
    }
}

public ReadOnlyDictionary<string, Action<T, object>> SetterCache
{
    get
    {
        return m_setterCache;
    }
}

Can become

public ReadOnlyCollection<string> CloneableProperties { get; }

public ReadOnlyDictionary<string, Func<T, object>> GetterCache { get; }

public ReadOnlyDictionary<string, Action<T, object>> SetterCache { get; }
  • Expression lambda
if (typeof(T).IsValueType)
{
    m_applyDefaultValues = (T instance) => { return defaultValue; };
    m_constructType = () => { return defaultValue; };
}
else if (defaultValue != null)
{
    var defaultConstructor = GetDefaultConstructor();
    var propertyValues = GetProperties(defaultValue, CloneableProperties).ToArray();

    m_applyDefaultValues = (T instance) =>
    {
        SetProperties(instance, propertyValues);
        return instance;
    };
    m_constructType = () => { return m_applyDefaultValues(defaultConstructor()); };
}
else
{
    m_applyDefaultValues = (T instance) => { return default(T); };
    m_constructType = () => { return default(T); };
}

Can become :

if (typeof(T).IsValueType)
{
    m_applyDefaultValues = instance => defaultValue;
    m_constructType = () => defaultValue;
}
else if (defaultValue != null)
{
    var defaultConstructor = GetDefaultConstructor();
    var propertyValues = GetProperties(defaultValue, CloneableProperties).ToArray();

    m_applyDefaultValues = instance =>
    {
        SetProperties(instance, propertyValues);
        return instance;
    };
    m_constructType = () => m_applyDefaultValues(defaultConstructor());
}
else
{
    m_applyDefaultValues = instance => default(T);
    m_constructType = () => default(T);
}
  • Interpolated string > String.Format

    You can make use of the interpolated string to shorten your exception handling:

throw new KeyNotFoundException(
    message:
    string.Format("a property setter with the name does not {0} exist on {1}", propertyName,
        typeof(T).FullName));
throw new KeyNotFoundException(
    message: $"a property setter with the name does not {propertyName} exist on {typeof(T).FullName}");
  • Execution branch outside the current block

    You have quite some redundant else statements e.g

if (type == typeof(string))
{
    return Expression.Lambda<Func<T>>(Expression.TypeAs(Expression.Constant(null),
typeof(string))).Compile();
}
else if (type.HasDefaultConstructor())
{
    return Expression.Lambda<Func<T>>(Expression.New(type)).Compile();
}
else {
    return () => (T)FormatterServices.GetUninitializedObject(type);
}

Can become

if (type == typeof(string))
{
    return Expression.Lambda<Func<T>>(Expression.TypeAs(Expression.Constant(null), typeof(string))).Compile();
}
if (type.HasDefaultConstructor())
{
    return Expression.Lambda<Func<T>>(Expression.New(type)).Compile();
}
return () => (T) FormatterServices.GetUninitializedObject(type);

Or a virtually one-liner with the ternary operator, tho it's quite hard to read :

return type == typeof(string)
    ? Expression.Lambda<Func<T>>(Expression.TypeAs(Expression.Constant(null), typeof(string))).Compile()
    : (type.HasDefaultConstructor()
        ? Expression.Lambda<Func<T>>(Expression.New(type)).Compile()
        : (() => (T) FormatterServices.GetUninitializedObject(type)));

You have the same "problem" in your GetProperties && GetProperty methods, I will leave the fixing of that up to you.

Alternative solution

Now let's move onto my alternative implementation, I really liked your original idea to have the properties with getter and setter in 2 separate dictionaries so I kept it that way, this result in a little bit longer version than yours but it's still a lot easier to use by the user.

public class NewTypeAccessor<T>
{
    private readonly Func<T, T> m_applyDefaultValues;
    private readonly Func<T> m_constructType;

    public ReadOnlyCollection<string> CloneableProperties { get; }

    public ReadOnlyDictionary<string, Func<T, object>> GetterCache { get; }

    public ReadOnlyDictionary<string, Action<T, object>> SetterCache { get; }


    public NewTypeAccessor(T defaultValue, bool includeNonPublic = false)
    {
        PropertyInfo[] properties = typeof(T).GetProperties(BindingFlags.Instance |
                        (includeNonPublic ? BindingFlags.NonPublic : BindingFlags.Default) |
                        BindingFlags.Public);

        GetterCache = new ReadOnlyDictionary<string, Func<T, object>>(properties.Select(propertyInfo => new
            {
                PropertyName = propertyInfo.Name,
                PropertyGetAccessor = propertyInfo.GetGetAccessor<T>(includeNonPublic)
            }).Where(a => a.PropertyGetAccessor != null)
            .ToDictionary(a => a.PropertyName, a => a.PropertyGetAccessor));

        SetterCache = new ReadOnlyDictionary<string, Action<T, object>>(properties.Select(propertyInfo => new
            {
                PropertyName = propertyInfo.Name,
                PropertySetAccessor = propertyInfo.GetSetAccessor<T>(includeNonPublic)
            }).Where(a => a.PropertySetAccessor != null)
            .ToDictionary(a => a.PropertyName, a => a.PropertySetAccessor));

        CloneableProperties = Array.AsReadOnly(GetterCache.Keys.Intersect(SetterCache.Keys).ToArray());


        if (typeof(T).IsValueType)
        {
            m_applyDefaultValues = instance => defaultValue;
            m_constructType = () => defaultValue;
        }
        else if (defaultValue != null)
        {
            var defaultConstructor = GetDefaultConstructor();
            var propertyValues = GetProperties(defaultValue, CloneableProperties).ToArray();

            m_applyDefaultValues = instance =>
            {
                SetProperties(instance, propertyValues);
                return instance;
            };
            m_constructType = () => m_applyDefaultValues(defaultConstructor());
        }
        else
        {
            m_applyDefaultValues = instance => default(T);
            m_constructType = () => default(T);
        }

    }
    public void CloneProperties(T source, T target)
    {
        SetProperties(target, GetProperties(source, CloneableProperties));
    }

    public void SetToDefault(ref T instance)
    {
        instance = m_applyDefaultValues(instance);
    }

    public T New()
    {
        return m_constructType();
    }

    private Dictionary<string, object> GetProperties(T instance, IEnumerable<string> properties)
        => properties?.ToDictionary(propertyName => propertyName,
               propertyName => GetProperty(instance, propertyName)) ?? new Dictionary<string, object>();

    public Dictionary<string, object> GetProperties(T instance,
        IEnumerable<Expression<Func<T, object>>> properties)
        => properties?.ToDictionary(property => GetMemberInfo(property).Name,
               property => GetProperty(instance, property)) ?? new Dictionary<string, object>();

    public Dictionary<string, object> GetProperties(T instance)
        => GetterCache.Keys.ToDictionary(key => key, key => GetProperty(instance, key));

    private object GetProperty(T instance, string propertyName) 
        => GetterCache[propertyName].Invoke(instance);

    public TValue GetProperty<TValue>(T instance, Expression<Func<T, TValue>> property)
        => (TValue) GetterCache[GetMemberInfo(property).Name](instance);

    private void SetProperty(T instance, string propertyName, object value)
    {
        Action<T, object> setter;

        if (SetterCache.TryGetValue(propertyName, out setter))
        {
            setter(instance, value);
        }
        else
        {
            throw new KeyNotFoundException(
                $"a property setter with the name does not {propertyName} exist on {typeof(T).FullName}");
        }
    }

    public void SetProperty<TValue>(T instance, Expression<Func<T, TValue>> property, TValue value)
        => SetterCache[GetMemberInfo(property).Name](instance, value);


    private void SetProperties<TValue>(T instance, IEnumerable<KeyValuePair<string, TValue>> properties)
    {
        if (properties != null)
        {
            foreach (var property in properties)
            {
                SetProperty(instance, property.Key, property.Value);
            }
        }
    }

    public void SetProperties<TValue>(T instance, IEnumerable<KeyValuePair<Expression<Func<T, TValue>>, TValue>> propertiesInfo)
    {
        foreach (var propertyInfo in propertiesInfo)
        {
            SetterCache[GetMemberInfo(propertyInfo.Key).Name](instance, propertyInfo.Value);
        }
    }

    private MemberInfo GetMemberInfo(Expression expression)
    {
        LambdaExpression lambda = (LambdaExpression)expression;
        MemberExpression memberExpr = null;
        switch (lambda.Body.NodeType)
        {
            case ExpressionType.Convert:
                memberExpr =
                    ((UnaryExpression)lambda.Body).Operand as MemberExpression;
                break;
            case ExpressionType.MemberAccess:
                memberExpr = lambda.Body as MemberExpression;
                break;
        }
        return memberExpr.Member;
    }

    private static Func<T> GetDefaultConstructor()
    {
        var type = typeof(T);

        if (type == typeof(string))
        {
            return Expression.Lambda<Func<T>>(Expression.TypeAs(Expression.Constant(null), typeof(string))).Compile();
        }
        if (type.HasDefaultConstructor())
        {
            return Expression.Lambda<Func<T>>(Expression.New(type)).Compile();
        }
        return () => (T)FormatterServices.GetUninitializedObject(type);
    }
}

Example usages :

public class Point2D
{
    public double X { get; set; }
    public double Y { get; set; }
}

var pointA = new Point2D {X = 9000.01, Y = 0.0};
var accessor = TypeAccessor.CreateNewTypeAccessor(pointA);
var pointB = new Point2D();

//obtains properties by name with compile time safety
Dictionary<string, object> a = accessor.GetProperties(pointA, new List<Expression<Func<Point2D, object>>>
{
    d => d.X,
    d => d.Y
});

accessor.CloneProperties(pointA, pointB); // pointB.X should now be 9000.01
accessor.SetProperty(pointA, p => p.X, 0.0);// sets pointA.X to 0.0
Console.WriteLine(accessor.GetProperty(pointA, p => p.X)); // prints pointA.X, should be 0.0
accessor.SetToDefault(ref pointA); // sets pointA's properties to default the accessor's default values
Console.WriteLine(accessor.GetProperty(pointA, p => p.X)); // prints pointA.X, should be 9000.01
\$\endgroup\$
5
  • \$\begingroup\$ I probably should've mentioned that this is primarily intended for use where the desired properties are not known until run-time. That said, providing a compile-time safe overload is a fantastic idea! The indexer was just something cute that I did but playing devil's advocate now: it does provide a "single point of entry" for a property and that could be useful... \$\endgroup\$ Dec 29, 2016 at 22:50
  • \$\begingroup\$ It's upto you but Indexer sounds to me like there is some collection to index, which is not the case here. \$\endgroup\$
    – Denis
    Dec 29, 2016 at 22:59
  • \$\begingroup\$ Sure there is, we're indexing into the collection of properties in the provided instance; it's just a dictionary that requires two keys to access. Does that make it more palatable? Also, would the variable name target be better as a whole than instance? \$\endgroup\$ Dec 29, 2016 at 23:03
  • \$\begingroup\$ Sounds better to me personally. There are few more variable names that I think should be changed CloneableProperties - those are not properties that's just a collection of property names and in the GetProperties function the IEnumerable<string> properties also has a misleading name again those are just names of some properties. \$\endgroup\$
    – Denis
    Dec 29, 2016 at 23:07
  • \$\begingroup\$ @Kittoes0124 providing a compile-time safe overload is a fantastic idea - I agree, but the performance may drasticly drop due to the expressions. It would be worth comparing both solutions as usually you want to use it in a generic loop where time is an important factor. \$\endgroup\$
    – t3chb0t
    Dec 30, 2016 at 6:29
2
\$\begingroup\$

I cannot complain about your code because it looks ok to me (at first glance) so I just share an idea.

You initialize all properties for an object at once.

I think it many scearios you use only a few of them so making the initialization process lazy should speed it up. A good start would be the Lazy<T> Class.

This means you initialize the getter/setter caches only if a property is accessed and even then, I would init only one cache at a time. If you use a getter, then init only the getter and so on.

The API wouldn't change, just the implemention of the constructor and GetProperty and SetProperty.

\$\endgroup\$
2
  • \$\begingroup\$ That's a very interesting idea to me. I'm not sure where I could actually place the lazy initialization though... I'm thinking the only way lazyness could actually help here is if I manage to change the signatures of GetGetAccessor and GetSetAccessor. Thoughts? \$\endgroup\$ Dec 29, 2016 at 16:59
  • \$\begingroup\$ @Kittoes0124 the initialization should occur or rather called by GetProperty or SetProperty if not yet initialized for this particular operation. \$\endgroup\$
    – t3chb0t
    Dec 29, 2016 at 17:14
1
\$\begingroup\$

Updates:

I have refactored class based on a combination of all of the feedback here, on the linked question, and a lot of benchmark results. The current implementation has been split into PropertyReader<T> and PropertyWriter<T> and drastically simplified from the original; a change I didn't like when first suggested to me but experimentation proved it to be superior in the end. The compile-time safety suggestions ended up being ignored simply because the original requirement was for string based access and one can always add such a helper via a simple extension.

Now, about performance. I was quite surprised when I loaded everything up and learned that @MarcGravell's implementation was roughly 4x faster than my own; I knew he'd win but I was shamed to lose THAT badly. It turns out that there were a lot of little details that were getting in the way.

I believed that one huge advantage he might have is that he's emitting IL to build a delegate while I was using expressions and so I spent the time figuring out how to emit the IL I needed. The problem? Once finished I only gained maybe 5 ns, he was beating me by at least 20 times that amount! This is when I learned that the slowest part of the entire process is not executing the delegate but extracting it from the cache.

I was using ImmutableDictionary<,> and wow is it slow! Really slow, even calling TryGetValue is at least 3 times slower than the normal Dictionary<,> class. Don't just take my word for it either as others have documented the same experience and I suppose it's not surprising giving the promises that the class has to fulfill. What surprised me more however is that TryGetValue itself is quite slow when compared to the pattern of try { return dictionary[key]; } catch (KeyNotFoundException e) { ... }.

Anyways, I was able to refine enough edges and end up with a property accessor that is consistently 3ns or so faster than FastMember. If my math and analysis of our code is correct then this delta is purely a result of the fact that he has a cast to object in his IL and I do not. The cast to object allows him to have a single point of entry and exit via a lovely indexer which I was forced to leave out because I wasn't clever enough to find a way to have my cake and eat it too.

Benchmark Results:

Benchmark Results

Benchmark Code:

using System;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

namespace ByteTerrace.CSharp.Sandbox
{
    class Point2D
    {
        public double X { get; set; }
        public double Y { get; set; }
    }

    class Program
    {
        static void Main(string[] args) {
            var summary = BenchmarkRunner.Run<PropertyAccessorComparison>();

            Console.ReadKey();
        }
    }

    public class PropertyAccessorComparison
    {
        private static readonly Point2D m_point = new Point2D();
        private static readonly PropertyReader<Point2D> m_propertyReader = new PropertyReader<Point2D>(includeStatic: true);
        private static readonly PropertyWriter<Point2D> m_propertyWriter = new PropertyWriter<Point2D>(includeStatic: true);
        private static readonly FastMember.TypeAccessor m_fastMember = FastMember.TypeAccessor.Create(typeof(Point2D));

        [Benchmark(Baseline = true)]
        public double NormalGet() {
            return m_point.X;
        }
        [Benchmark]
        public object FastMemberGet() {
            return m_fastMember[m_point, "X"];
        }
        [Benchmark]
        public void FastMemberSet() {
            m_fastMember[m_point, "X"] = 9000.01;
        }
        [Benchmark]
        public double PropertyReaderGet() {
            return m_propertyReader.GetValue<double>(m_point, "X");
        }
        [Benchmark]
        public void PropertyWriterSet() {
            m_propertyWriter.SetValue(m_point, "X", 9000.01);
        }
    }
}

PropertyReader<T>:

using System;
using System.Collections.Generic;
using System.Reflection;
using System.Reflection.Emit;

namespace ByteTerrace.CSharp
{
    /// <summary>
    /// Provides cached access to a type's property getters.
    /// </summary>
    public interface IPropertyReader<T>
    {
        /// <summary>
        /// Returns a read-only cache of property getters that belong to T.
        /// </summary>
        IReadOnlyDictionary<string, Delegate> GetterCache { get; }

        /// <summary>
        /// Uses the <see cref="IPropertyReader{_}"/> to get the value of a property by name.
        /// </summary>
        /// <param name="instance">The instance of T to get a property from.</param>
        /// <param name="propertyName">The name of the property to retrieve from the instance of T.</param>
        TProperty GetValue<TProperty>(T instance, string propertyName);
    }

    /// <summary>
    /// Provides cached access to a type's property getters.
    /// </summary>
    public class PropertyReader<T> : IPropertyReader<T>
    {
        private readonly BindingFlags m_bindingFlags;

        /// <summary>
        /// Returns a read-only cache of property getters that belong to T.
        /// </summary>
        public IReadOnlyDictionary<string, Delegate> GetterCache { get; } = new Dictionary<string, Delegate>();

        /// <summary>
        /// Initializes a new instance of the <see cref="PropertyReader{_}"/> class.
        /// </summary>
        /// <param name="includePublic">Indicates whether public properties should be included.</param>
        /// <param name="includeNonPublic">Indicates whether non-public properties should be included.</param>
        /// <param name="includeInstance">Indicates whether instance properties should be included.</param>
        /// <param name="includeStatic">Indicates whether static properties should be included.</param>
        public PropertyReader(bool includePublic = true, bool includeNonPublic = false, bool includeInstance = true, bool includeStatic = false) {
            m_bindingFlags =
                (includeInstance ? BindingFlags.Instance : BindingFlags.Default)
                | (includeNonPublic ? BindingFlags.NonPublic : BindingFlags.Default)
                | (includePublic ? BindingFlags.Public : BindingFlags.Default)
                | (includeStatic ? BindingFlags.Static : BindingFlags.Default);
        }

        /// <summary>
        /// Uses the <see cref="PropertyReader{_}"/> to get the value of a property by name.
        /// </summary>
        /// <param name="instance">The instance of T to get a property from.</param>
        /// <param name="propertyName">The name of the property to retrieve from the instance of T.</param>
        public TProperty GetValue<TProperty>(T instance, string propertyName) {
            Delegate getter;

            try {
                getter = GetterCache[propertyName];
            }
            catch (KeyNotFoundException) {
                var type = typeof(T);
                var propertyInfo = type.GetProperty(propertyName, m_bindingFlags);

                if ((propertyInfo != null) && (getter = GetGetAccessor(propertyInfo, m_bindingFlags.HasFlag(BindingFlags.NonPublic))) != null) {
                    ((IDictionary<string, Delegate>)GetterCache).Add(propertyName, getter);
                }
                else {
                    throw new KeyNotFoundException($"a property getter with the name {propertyName} could not be found on {type.FullName}");
                }
            }

            return ((Func<T, TProperty>)getter)(instance);
        }

        /// <summary>
        /// Generates a <see cref="Func{_,_}"/> delegate that represents the <see cref="PropertyInfo"/>'s getter.
        /// </summary>
        /// <param name="propertyInfo">The <see cref="PropertyInfo"/> instance to extract a getter from.</param>
        /// <param name="includeNonPublic">Indicates whether a non-public get accessor should be returned.</param>
        public static Delegate GetGetAccessor(PropertyInfo propertyInfo, bool includeNonPublic = false) {
            if (propertyInfo == null) { throw new ArgumentNullException(nameof(propertyInfo)); }
            if (propertyInfo.GetIndexParameters().Length > 0) { throw new NotImplementedException("indexer properties are not supported"); }

            var getMethod = propertyInfo.GetGetMethod(includeNonPublic);

            if (getMethod != null) {
                var getMethodDynamicCall = new DynamicMethod(
                    string.Concat(getMethod.Name, "_DynamicGetter_", Guid.NewGuid().ToString("N").ToUpper()),
                    propertyInfo.PropertyType,
                    new[] { propertyInfo.DeclaringType },
                    propertyInfo.DeclaringType,
                    true
                );
                var il = getMethodDynamicCall.GetILGenerator();

                if (!getMethod.IsStatic) {
                    il.Emit(OpCodes.Ldarg_0);
                }
                il.EmitCall(OpCodes.Call, getMethod, null);
                il.Emit(OpCodes.Ret);

                return getMethodDynamicCall.CreateDelegate(typeof(Func<,>).MakeGenericType(propertyInfo.DeclaringType, propertyInfo.PropertyType));
            }
            else {
                return null;
            }
        }
    }
}

PropertyWriter<T>:

using System;
using System.Collections.Generic;
using System.Reflection;
using System.Reflection.Emit;

namespace ByteTerrace.CSharp
{
    /// <summary>
    /// Provides cached access to a type's property setters.
    /// </summary>
    public interface IPropertyWriter<T>
    {
        /// <summary>
        /// Returns a read-only cache of property setters that belong to T.
        /// </summary>
        IReadOnlyDictionary<string, Delegate> SetterCache { get; }

        /// <summary>
        /// Uses the <see cref="IPropertyWriter{_}"/> to set the value of a property by name.
        /// </summary>
        /// <param name="instance">The instance of T to set a property value for.</param>
        /// <param name="propertyName">The name of the property to alter on the instance of T.</param>
        /// <param name="value">The value that will be applied to the property.</param>
        void SetValue<TProperty>(T instance, string propertyName, TProperty value);
    }

    /// <summary>
    /// Provides cached access to a type's property setters.
    /// </summary>
    public class PropertyWriter<T> : IPropertyWriter<T>
    {
        private readonly BindingFlags m_bindingFlags;

        /// <summary>
        /// Returns a read-only cache of property setters that belong to T.
        /// </summary>
        public IReadOnlyDictionary<string, Delegate> SetterCache { get; } = new Dictionary<string, Delegate>();

        /// <summary>
        /// Initializes a new instance of the <see cref="PropertyWriter{_}"/> class.
        /// </summary>
        /// <param name="includePublic">Indicates whether public properties should be included.</param>
        /// <param name="includeNonPublic">Indicates whether non-public properties should be included.</param>
        /// <param name="includeInstance">Indicates whether instance properties should be included.</param>
        /// <param name="includeStatic">Indicates whether static properties should be included.</param>
        public PropertyWriter(bool includePublic = true, bool includeNonPublic = false, bool includeInstance = true, bool includeStatic = false) {
            m_bindingFlags =
                (includeInstance ? BindingFlags.Instance : BindingFlags.Default)
                | (includeNonPublic ? BindingFlags.NonPublic : BindingFlags.Default)
                | (includePublic ? BindingFlags.Public : BindingFlags.Default)
                | (includeStatic ? BindingFlags.Static : BindingFlags.Default);
        }

        /// <summary>
        /// Uses the <see cref="PropertyWriter{_}"/> to set the value of a property by name.
        /// </summary>
        /// <param name="instance">The instance of T to set a property value for.</param>
        /// <param name="propertyName">The name of the property to alter on the instance of T.</param>
        /// <param name="value">The value that will be applied to the property.</param>
        public void SetValue<TProperty>(T instance, string propertyName, TProperty value) {
            Delegate setter;

            try {
                setter = SetterCache[propertyName];
            }
            catch (KeyNotFoundException) {
                var type = typeof(T);
                var propertyInfo = type.GetProperty(propertyName, m_bindingFlags);

                if ((propertyInfo != null) && (setter = GetSetAccessor(propertyInfo, m_bindingFlags.HasFlag(BindingFlags.NonPublic))) != null) {
                    ((IDictionary<string, Delegate>)SetterCache).Add(propertyName, setter);
                }
                else {
                    throw new KeyNotFoundException($"a property setter with the name {propertyName} could not be found on {type.FullName}");
                }
            }

            ((Action<T, TProperty>)setter)(instance, value);
        }

        /// <summary>
        /// Generates an <see cref="Action{_,_}"/> delegate that represents the <see cref="PropertyInfo"/>'s setter.
        /// </summary>
        /// <param name="propertyInfo">The <see cref="PropertyInfo"/> instance to extract a setter from.</param>
        /// <param name="includeNonPublic">Indicates whether a non-public set accessor should be returned.</param>
        public static Delegate GetSetAccessor(PropertyInfo propertyInfo, bool includeNonPublic = false) {
            if (propertyInfo == null) { throw new ArgumentNullException(nameof(propertyInfo)); }
            if (propertyInfo.GetIndexParameters().Length > 0) { throw new NotImplementedException("indexer properties are not supported"); }

            var setMethod = propertyInfo.GetSetMethod(includeNonPublic);

            if (setMethod != null) {
                var setMethodDynamicCall = new DynamicMethod(
                    string.Concat(setMethod.Name, "_DynamicSetter_", Guid.NewGuid().ToString("N").ToUpper()),
                    null,
                    new[] { propertyInfo.DeclaringType, propertyInfo.PropertyType },
                    propertyInfo.DeclaringType,
                    true
                );
                var il = setMethodDynamicCall.GetILGenerator();

                if (!setMethod.IsStatic) {
                    il.Emit(OpCodes.Ldarg_0);
                    il.Emit(OpCodes.Ldarg_1);
                }
                else {
                    il.Emit(OpCodes.Ldarg_1);
                }
                il.EmitCall(OpCodes.Call, setMethod, null);
                il.Emit(OpCodes.Ret);

                return setMethodDynamicCall.CreateDelegate(typeof(Action<,>).MakeGenericType(propertyInfo.DeclaringType, propertyInfo.PropertyType));
            }
            else {
                return null;
            }
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.