4

I tried to implement Jon Skeet's solution for this question posted on this blog post to substitute the SetValue method with a non-reflection method using delegates.

The difference from the solution in the blog post is that SetValue is void, and I get the The type 'System.Void' may not be used as a type argument. exception at the line MethodInfo miConstructedHelper = miGenericHelper.MakeGenericMethod(typeof(G), pMethod.GetParameters()[0].ParameterType, pMethod.ReturnType);.

Here's my implementation of the MagicMethod:

public class Instantiator<T> where T : new()
{
    private T instance;
    private IDictionary<string, PropertyInfo> properties;

    private Func<PropertyInfo, object, object> _fncSetValue;

    public Instantiator()
    {
        Type type = typeof(T);
        properties = type.GetProperties().GroupBy(p => p.Name).ToDictionary(g => g.Key, g => g.ToList().First());

        MethodInfo miSetValue = typeof(PropertyInfo).GetMethod("SetValue", new Type[] { typeof(object), typeof(object), typeof(object[]) });
        _fncSetValue = SetValueMethod<PropertyInfo>(miSetValue);
    }

    public void CreateNewInstance()
    {
        instance = new T();
    }

    public void SetValue(string pPropertyName, object pValue)
    {
        if (pPropertyName == null) return;
        PropertyInfo property;
        if (!properties.TryGetValue(pPropertyName, out property)) return;
        TypeConverter tc = TypeDescriptor.GetConverter(property.PropertyType);

        //substitute this line
        //property.SetValue(instance, tc.ConvertTo(pValue, property.PropertyType), null);
        //with this line
        _fncSetValue(property, new object[] { instance, tc.ConvertTo(pValue, property.PropertyType), null });
    }

    public T GetInstance()
    {
        return instance;
    }

    private static Func<G, object, object> SetValueMethod<G>(MethodInfo pMethod) where G : class
    {
        MethodInfo miGenericHelper = typeof(Instantiator<T>).GetMethod("SetValueMethodHelper", BindingFlags.Static | BindingFlags.NonPublic);
        MethodInfo miConstructedHelper = miGenericHelper.MakeGenericMethod(typeof(G), pMethod.GetParameters()[0].ParameterType, pMethod.ReturnType);
        object retVal = miConstructedHelper.Invoke(null, new object[] { pMethod });
        return (Func<G, object, object>) retVal;
    }

    private static Func<TTarget, object, object> SetValueMethodHelper<TTarget, TParam, TReturn>(MethodInfo pMethod) where TTarget : class
    {
        Func<TTarget, TParam, TReturn> func = (Func<TTarget, TParam, TReturn>)Delegate.CreateDelegate(typeof(Func<TTarget, TParam, TReturn>), pMethod);
        Func<TTarget, object, object> retVal = (TTarget target, object param) => func(target, (TParam) param);
        return retVal;
    }
}
Community
  • 1
  • 1
Mentoliptus
  • 2,855
  • 3
  • 27
  • 35

1 Answers1

6

You are using Func in your code. Func is for methods that have a return type. For methods that return void you need to use Action.


Your code needs to look like this:

public class Instantiator<T> where T : new()
{
    private T instance;
    private IDictionary<string, PropertyInfo> properties;

    private Action<PropertyInfo, object, object, object> _fncSetValue;

    public Instantiator()
    {
        Type type = typeof(T);
        properties = type.GetProperties()
                         .GroupBy(p => p.Name)
                         .ToDictionary(g => g.Key, g => g.ToList().First());

        var types = new Type[] { typeof(object), typeof(object),
                                 typeof(object[]) };
        var miSetValue = typeof(PropertyInfo).GetMethod("SetValue", types);
        _fncSetValue = SetValueMethod<PropertyInfo>(miSetValue);
    }

    public void CreateNewInstance()
    {
        instance = new T();
    }

    public void SetValue(string pPropertyName, object pValue)
    {
        if (pPropertyName == null) return;
        PropertyInfo property;
        if (!properties.TryGetValue(pPropertyName, out property)) return;
        TypeConverter tc = TypeDescriptor.GetConverter(property.PropertyType);

        var value = tc.ConvertTo(pValue, property.PropertyType);
        _fncSetValue(property, instance, value, null);
    }

    public T GetInstance()
    {
        return instance;
    }

    private static Action<G, object, object, object> SetValueMethod<G>(MethodInfo pMethod) where G : class
    {
        var miGenericHelper = 
            typeof(Instantiator<T>).GetMethod("SetValueMethodHelper", 
                                              BindingFlags.Static | 
                                              BindingFlags.NonPublic);

        var parameters = pMethod.GetParameters();
        var miConstructedHelper = miGenericHelper.MakeGenericMethod(typeof(G), 
                                      parameters[0].ParameterType,
                                      parameters[1].ParameterType,
                                      parameters[2].ParameterType);

        var retVal = miConstructedHelper.Invoke(null, new object[] { pMethod });
        return (Action<G, object, object, object>) retVal;
    }

    private static Action<TTarget, object, object, object> SetValueMethodHelper<TTarget, TParam1, TParam2, TParam3>(MethodInfo pMethod) where TTarget : class
    {
        var func = (Action<TTarget, TParam1, TParam2, TParam3>)Delegate.CreateDelegate(typeof(Action<TTarget, TParam1, TParam2, TParam3>), pMethod);
        Action<TTarget, object, object, object> retVal =
            (target, param1, param2, param3) => 
                func(target, (TParam1) param1, (TParam2) param2, (TParam3) param3);

        return retVal;
    }
}

As you don't want to call arbitrary methods like Jon Skeet, you can simplify your code a lot. There is no need for a call to MethodInfo.Invoke in your code and because of this there is no need for the delegates. You can simply call SetValue directly on the returned PropertyInfo. There is no need to use the detour of a delegate that in turn calls exactly that method anyway. Additionally, the type conversion is not necessary as SetValue requires an object anyway.
Your code could be as simple as this:

public class SimpleInstantiator<T> where T : new()
{
    private T instance;
    private IDictionary<string, PropertyInfo> properties;

    public SimpleInstantiator()
    {
        Type type = typeof(T);
        properties = type.GetProperties()
                         .GroupBy(p => p.Name)
                         .ToDictionary(g => g.Key, g => g.ToList().First());
    }

    public void CreateNewInstance()
    {
        instance = new T();
    }

    public void SetValue(string pPropertyName, object pValue)
    {
        if (pPropertyName == null) return;

        PropertyInfo property;
        if (!properties.TryGetValue(pPropertyName, out property)) return;

        property.SetValue(instance, pValue, null);
    }

    public T GetInstance()
    {
        return instance;
    }
}

Performance tests show that this version takes only about 50% of the previous one.
A tiny increase in performance is due to the fact that we avoid two unnecessary delegates in our call chain. However, the vast majority of the speed improvement lays in the fact that we removed the type conversion.

Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
  • Thanks, this solved the above mentioned exception. But now I get `Error binding to target method.` on the line `MethodInfo miConstructedHelper = miGenericHelper.MakeGenericMethod(typeof(G), pMethod.GetParameters()[0].ParameterType, pMethod.ReturnType);` I think it's because `SetValue` has more than one parameter... – Mentoliptus Oct 19 '12 at 07:45
  • 1
    @Mentoliptus: You need to remove `pMethod.ReturnType`. And you need to remove the generic argument `TReturn` from `SetValueMethodHelper`. – Daniel Hilgarth Oct 19 '12 at 07:46
  • I did that, that part works fine. The `MakeGenericMethod` accepts a Type[] of arguments, one target and the other for the method parameter. The method in the blog post has one parameter, hence the `pMethod.GetParameters()[0].ParameterType`, but I have 3 parameters. – Mentoliptus Oct 19 '12 at 07:53
  • Where do you have three parameters? – Daniel Hilgarth Oct 19 '12 at 07:54
  • I added `, pMethod.GetParameters()[1].ParameterType, pMethod.GetParameters()[2].ParameterType` but didn't figure I had to add the params to all the `Action` declarations. Thank you very very much! I learned a lot here. – Mentoliptus Oct 19 '12 at 08:37
  • @Mentoliptus: You are welcome. But please have a look at the second update. I show a way that doesn't need these delegates at all. – Daniel Hilgarth Oct 19 '12 at 08:57
  • Yes, your second update is very similar to the code that I got to "optimize", because the reflection call of `SetValue` was slow. So I had to do "something" and the best thing I found was the solution with `Delegate`. Thanks again! – Mentoliptus Oct 19 '12 at 09:15
  • @Mentoliptus: But my second update is a *lot* faster than your "optimized" version...! – Daniel Hilgarth Oct 19 '12 at 09:17
  • PropertyInfo.SetValue is slow, and using a delegate is much faster. Not a delegate that calls PropertyInfo.SetValue (which is obviously and extra step and slower), but a delegate that calls the setter or getter of the property directly, such as: HyperDescriptor, and two versions of FastInvoke (http://flurfunk.sdx-ag.de/2012/05/c-performance-bei-der-befullungmapping.html and https://code.google.com/p/somemethods/source/browse/trunk/DotNet/Dotnet.Invoke/FastInvoke.cs?r=37) – Triynko Oct 21 '13 at 01:37