55

I asked a question yesterday regarding using either reflection or Strategy Pattern for dynamically calling methods.

However, since then I have decided to change the methods into individual classes that implement a common interface. The reason being, each class, whilst bearing some similarities also perform certain methods unique to that class.

I had been using a strategy as such:

switch (method)
{
    case "Pivot":
        return new Pivot(originalData);
    case "GroupBy":
        return new GroupBy(originalData);
    case "Standard deviation":
        return new StandardDeviation(originalData);
    case "% phospho PRAS Protein":
        return new PhosphoPRASPercentage(originalData);
    case "AveragePPPperTreatment":
        return new AveragePPPperTreatment(originalData);
    case "AvgPPPNControl":
        return new AvgPPPNControl(originalData);
    case "PercentageInhibition":
        return new PercentageInhibition(originalData);
    default:
        throw new Exception("ERROR: Method " + method + " does not exist.");
}

However, as the number of potential classes grow, I will need to keep adding new ones, thus breaking the closed for modification rule.

Instead, I have used a solution as such:

var test = Activator.CreateInstance(null, "MBDDXDataViews."+ _class);
       ICalculation instance = (ICalculation)test.Unwrap();
       return instance;

Effectively, the _class parameter is the name of the class passed in at runtime. Is this a common way to do this, will there be any performance issues with this?

I am fairly new to reflection, so your advice would be welcome.

Abel
  • 56,041
  • 24
  • 146
  • 247
Darren Young
  • 10,972
  • 36
  • 91
  • 150
  • Don't use Activator.CreateInstance, it's used with marshaling and it creates a wrapper object that isn't equal to the original object. Instead, use reflection. PS What version of .NET do you use? – Abel Mar 10 '11 at 19:13
  • 1
    As an alternative to my approach, you may consider Vitaliy's approach as well. It may look daunting at first, but it uses a different approach to encapsulating methods and ctors using an Expression builder. It can add extra benefits in terms of flexibility (when you need it). – Abel Mar 11 '11 at 11:37
  • @Abel - thanks I will have a go at implementing that as well,when I get a little time. – Darren Young Mar 11 '11 at 11:46

6 Answers6

79

When using reflection you should ask yourself a couple of questions first, because you may end up in an over-the-top complex solution that's hard to maintain:

  1. Is there a way to solve the problem using genericity or class/interface inheritance?
  2. Can I solve the problem using dynamic invocations (only .NET 4.0 and above)?
  3. Is performance important, i.e. will my reflected method or instantiation call be called once, twice or a million times?
  4. Can I combine technologies to get to a smart but workable/understandable solution?
  5. Am I ok with losing compile time type safety?

Genericity / dynamic

From your description I assume you do not know the types at compile time, you only know they share the interface ICalculation. If this is correct, then number (1) and (2) above are likely not possible in your scenario.

Performance

This is an important question to ask. The overhead of using reflection can impede a more than 400-fold penalty: that slows down even a moderate amount of calls.

The resolution is relatively easy: instead of using Activator.CreateInstance, use a factory method (you already have that), look up the MethodInfo create a delegate, cache it and use the delegate from then on. This yields only a penalty on the first invocation, subsequent invocations have near-native performance.

Combine technologies

A lot is possible here, but I'd really need to know more of your situation to assist in this direction. Often, I end up combining dynamic with generics, with cached reflection. When using information hiding (as is normal in OOP), you may end up with a fast, stable and still well-extensible solution.

Losing compile time type safety

Of the five questions, this is perhaps the most important one to worry about. It is very important to create your own exceptions that give clear information about reflection mistakes. That means: every call to a method, constructor or property based on an input string or otherwise unchecked information must be wrapped in a try/catch. Catch only specific exceptions (as always, I mean: never catch Exception itself).

Focus on TargetException (method does not exist), TargetInvocationException (method exists, but rose an exc. when invoked), TargetParameterCountException, MethodAccessException (not the right privileges, happens a lot in ASP.NET), InvalidOperationException (happens with generic types). You don't always need to try to catch all of them, it depends on the expected input and expected target objects.

To sum it up

Get rid of your Activator.CreateInstance and use MethodInfo to find the factory-create method, and use Delegate.CreateDelegate to create and cache the delegate. Simply store it in a static Dictionary where the key is equal to the class-string in your example code. Below is a quick but not-so-dirty way of doing this safely and without losing too much type safety.

Sample code

public class TestDynamicFactory
{
    // static storage
    private static Dictionary<string, Func<ICalculate>> InstanceCreateCache = new Dictionary<string, Func<ICalculate>>();

    // how to invoke it
    static int Main()
    {
        // invoke it, this is lightning fast and the first-time cache will be arranged
        // also, no need to give the full method anymore, just the classname, as we
        // use an interface for the rest. Almost full type safety!
        ICalculate instanceOfCalculator = this.CreateCachableICalculate("RandomNumber");
        int result = instanceOfCalculator.ExecuteCalculation();
    }

    // searches for the class, initiates it (calls factory method) and returns the instance
    // TODO: add a lot of error handling!
    ICalculate CreateCachableICalculate(string className)
    {
        if(!InstanceCreateCache.ContainsKey(className))
        {
            // get the type (several ways exist, this is an eays one)
            Type type = TypeDelegator.GetType("TestDynamicFactory." + className);

            // NOTE: this can be tempting, but do NOT use the following, because you cannot 
            // create a delegate from a ctor and will loose many performance benefits
            //ConstructorInfo constructorInfo = type.GetConstructor(Type.EmptyTypes);

            // works with public instance/static methods
            MethodInfo mi = type.GetMethod("Create");

            // the "magic", turn it into a delegate
            var createInstanceDelegate = (Func<ICalculate>) Delegate.CreateDelegate(typeof (Func<ICalculate>), mi);

            // store for future reference
            InstanceCreateCache.Add(className, createInstanceDelegate);
        }

        return InstanceCreateCache[className].Invoke();

    }
}

// example of your ICalculate interface
public interface ICalculate
{
    void Initialize();
    int ExecuteCalculation();
}

// example of an ICalculate class
public class RandomNumber : ICalculate
{
    private static Random  _random;

    public static RandomNumber Create()
    {
        var random = new RandomNumber();
        random.Initialize();
        return random;
    }

    public void Initialize()
    {
        _random = new Random(DateTime.Now.Millisecond);
    }

    public int ExecuteCalculation()
    {
        return _random.Next();
    }
}
Abel
  • 56,041
  • 24
  • 146
  • 247
  • Thanks for the excellent answer. The 'magic' is quite confusing though , would you mind explaining how that works if you don't mind? Thanks. – Darren Young Mar 11 '11 at 09:49
  • 1
    I don't really see, how a method using reflection and magic strings can be better than a type safe and compile time checkable one as provided by me. Please explain - this is an honest question, because I seem to be missing something. – Daniel Hilgarth Mar 11 '11 at 10:02
  • @Daniel, apologies if I caused offence. I accepted this answer due to the overall verbosity of the reply, not simply the code sample. To be honest, I am not yet knowledgeable to know which code sample is the more efficient and why, hence my decision. – Darren Young Mar 11 '11 at 10:13
  • @Darren Young: That's no problem and no offense taken. As I said, it was an honest question from me and more directed into @Abel's direction. – Daniel Hilgarth Mar 11 '11 at 10:17
  • @Daniel Hilgarth: I focused on how to go from a string to an instantiated class, using reflection (the q. was about reflection, but OP did not contain actual reflection). Your answer fits well with my answer in certain respects, but is unfortunately not possible, because you don't know T. I will add a comment under your thread, it is more suitable there. – Abel Mar 11 '11 at 10:49
  • @Abel, I am getting an error with the line var createInstanceDelegate = (Func)Delegate.CreateDelegate(typeof(Func), mi); It says 'ERROR binding to target method'. The method takes two parameters, so I have declared mi both without any parameter list and with a parameter list as such: Type[] paramTypes = new Type[]{typeof(DataTable), typeof(System.String[])}; MethodInfo mi = type.GetMethod("Calculate", paramTypes); However, I get the same error on both counts. Any ideas? Thanks. – Darren Young Mar 11 '11 at 11:06
  • @Darren: Many ideas. `Func` should be seen as "function that returns `T` (the first generic param), and takes parameters `U`, `V` etc. Suppose `originalData` is of type `OriginalData` then change your code to use `Func` and make sure that `Create` is static, takes one param of type `OriginalData` and returns an instance of the containing class (the constructors in your code). – Abel Mar 11 '11 at 11:12
  • +1 for pointing out the problems involved with the reflection approach. – Daniel Hilgarth Mar 11 '11 at 11:21
  • @Darren: a little explanation on the magic. Every delegate in C# can hold a method. `Func<...>` and `Action<...>` are helper-classes that take away the effort of declaring your own delegate. `CreateDelegate` can take a MethodInfo object (which you can find for _every_ method of a class using reflection) and turn that into an actual delegate (which is almost equal to an actual method). Delegates can get rather advanced and confusing, but here's an excellent article explaining the basics: http://www.akadia.com/services/dotnet_delegates_and_events.html – Abel Mar 11 '11 at 11:28
  • @Abel, works fine now. I have the static create method, that simply returns the object. I then call the interface method ("calculate"), when the object is returned. Works like a charm. Thanks for the help. – Darren Young Mar 11 '11 at 11:39
  • @Abel - curious, instead of creating the Delegate, couldn't you just cache the MethodInfo and call invoke on that? – Suraj Mar 11 '11 at 17:36
  • @SFun yes, you can. But Invoke of MethodInfo is much, much slower than using a Delegate, and a Delegate is type-safe, Invoke is not (and, if it matters, Delegates are easier to work with, you can use them as if they are functions). – Abel Mar 11 '11 at 17:44
  • Good explaination. This is very similar to how Dependency Injection framworks work. You could use a DI Framework to solve the problem. – Chuck Conway Jun 20 '12 at 19:37
  • As an alternative you can also create an Expression Tree, compile it (for reuse) and get a Func<> (delegate). For a one time use you can just evaluate the expression tree. Expressions Trees are faster than using Reflection, althouth it could more difficult to understand and build expression trees at start but once you get inside of it are great for this kind of operations. – nflash Oct 05 '15 at 17:18
  • I have a custom validation attribute which is applied on `Property A` and has an IsValid() method(_obviously_). I need to get the value of another property say, `Property B`. I need explicit casting for `(ClassName)validationContext.ObjectInstance` to get the value. I am using `Activator.CreateInstance(validationContext.ObjectType)` to get my class `ClassName`. Is it correct to use it, because as you stated a̶s̶ ̶i̶t̶ ̶c̶o̶m̶e̶s̶ ̶a̶t̶ ̶a̶ ̶c̶o̶s̶t̶ ̶o̶f̶ ̶p̶e̶r̶f̶o̶r̶m̶a̶n̶c̶e̶. Is there any other way to do this without using `Activator.CreateInstance(validationContext.ObjectType)`? – phougatv Jun 28 '17 at 13:04
  • 1
    @barnes, if performance is important, then do not use `Activator.CreateInstance`. There are many other ways. You should probably ask a separate question about it, if my answer above doesn't give enough for you to work with. – Abel Jun 28 '17 at 14:24
18

I suggest you give your factory implementation a method RegisterImplementation. So every new class is just a call to that method and you are not changing your factories code.

UPDATE:
What I mean is something like this:

Create an interface that defines a calculation. According to your code, you already did this. For the sake of being complete, I am going to use the following interface in the rest of my answer:

public interface ICalculation
{
    void Initialize(string originalData);
    void DoWork();
}

Your factory will look something like this:

public class CalculationFactory
{
    private readonly Dictionary<string, Func<string, ICalculation>> _calculations = 
                        new Dictionary<string, Func<string, ICalculation>>();

    public void RegisterCalculation<T>(string method)
        where T : ICalculation, new()
    {
        _calculations.Add(method, originalData =>
                                  {
                                      var calculation = new T();
                                      calculation.Initialize(originalData);
                                      return calculation;
                                  });
    }

    public ICalculation CreateInstance(string method, string originalData)
    {
        return _calculations[method](originalData);
    }
}

This simple factory class is lacking error checking for the reason of simplicity.

UPDATE 2:
You would initialize it like this somewhere in your applications initialization routine:

CalculationFactory _factory = new CalculationFactory();

public void RegisterCalculations()
{
    _factory.RegisterCalculation<Pivot>("Pivot");
    _factory.RegisterCalculation<GroupBy>("GroupBy");
    _factory.RegisterCalculation<StandardDeviation>("Standard deviation");
    _factory.RegisterCalculation<PhosphoPRASPercentage>("% phospho PRAS Protein");
    _factory.RegisterCalculation<AveragePPPperTreatment>("AveragePPPperTreatment");
    _factory.RegisterCalculation<AvgPPPNControl>("AvgPPPNControl");
    _factory.RegisterCalculation<PercentageInhibition>("PercentageInhibition");
}
Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
  • Yes, the Activator code I written above is within a method that just returns the instance. I just call that when I want a new object. Is that what you meant? – Darren Young Mar 10 '11 at 16:45
  • @Darren Young: No, that is not what I meant. I updated my answer to be more precise on what I meant. – Daniel Hilgarth Mar 10 '11 at 19:26
  • @Daniel, I've tried implementing your code, however I don't think it will work for my case. I cannot supply the generic T type as the only thing I have at compile time are strings that relate to the class names, not the actual Types themselves. – Darren Young Mar 11 '11 at 10:49
  • 1
    (in answer to your question under my answer) Unfortunately, this won't work. To use `RegisterCalculation` you will still need a large switch-statement like in the OP. The idea was (as I understood it) to remove the dependency between string and classname. This dependency is still here. You win (more) type safety, agreed, but you don't solve the original problem. (oops, see that Darren came to the same conclusion). – Abel Mar 11 '11 at 10:58
  • @Darren Young & @Abel: Please see the second update to my question for the intended usage, in case it wasn't clear already. No switch is needed. As far as I understand your question, Darren, you **do** have the actual types at compile time - you are using them in your switch statement. I understood your question like this: *"How do I avoid that ugly switch statement and is my solution using reflection a good one?"* Maybe I just misunderstood the question. :-) – Daniel Hilgarth Mar 11 '11 at 11:00
  • @Daniel - I think there has been some confusion. However, I have still learned from your answer and will no doubt find this useful in the future. Thanks :) – Darren Young Mar 11 '11 at 11:04
  • @Darren Young: If you learned something from it, it was worth the effort. I am still not sure, whether your architecture is a good one, when it relies on magic strings. Please do yourself the favor and review it with both Abel's and my answer in mind. – Daniel Hilgarth Mar 11 '11 at 11:17
  • @Daniel: perfect! Assuming the type is available at compile time indeed, this makes excellent sense (requiring the Register-line instead of the switch-line, though, but it is much clearer and less error-prone). Thanks for expanding. – Abel Mar 11 '11 at 11:18
  • @Abel: Sure, if you have a key, you need SOME way to do the mapping. If you want to go without, you will have to do the magic in your answer. But also in that case, the string is just a key, but now with the restriction that it must match the class name. – Daniel Hilgarth Mar 11 '11 at 11:23
  • @Daniel: yes indeed! ;). One more tiny thing that is of importance in terms of encapsulation: if the constructor is private or otherwise hidden, your code will still fail. The approach with a static Create-method can use private ctors, which can be a benefit (but losing type safety is a huge disadvantage). – Abel Mar 11 '11 at 11:31
  • @Abel: If the constructor is not public there is a reason to it. IMHO it is not an advantage but a disadvantage that the method using reflection can work around that. – Daniel Hilgarth Mar 11 '11 at 13:11
  • @Daniel, sorry if you misunderstood me, but, while reflection _can_ work around that, my solution does _not_ break information hiding. It calls a static `Create` method, similar to `XmlWriter.Create`, which keeps the internal creation-code from the user, and you shouldn't care about the actual internal object returned. That's not working _around_, but working _with_ information hiding (ps: interesting discussion altogether!). – Abel Mar 11 '11 at 13:25
  • 2
    @Abel: My error, sorry. You are right, that you are not breaking information hiding. You are right, this is a good discussion about an interesting topic, I *really* hope, we are not the only ones, reading it! – Daniel Hilgarth Mar 11 '11 at 13:30
7

Just as an example how to add initialization in the constructor:

Something similar to:

Activator.CreateInstance(Type.GetType("ConsoleApplication1.Operation1"), initializationData);
but written with Linq Expression, part of code is taken here:
public class Operation1 
{
    public Operation1(object data)
    { 
    }
}

public class Operation2 
{
    public Operation2(object data)
    {
    }
}

public class ActivatorsStorage
{
    public delegate object ObjectActivator(params object[] args);

    private readonly Dictionary<string, ObjectActivator> activators = new Dictionary<string,ObjectActivator>();

    private ObjectActivator CreateActivator(ConstructorInfo ctor)
    {
        Type type = ctor.DeclaringType;

        ParameterInfo[] paramsInfo = ctor.GetParameters();
        ParameterExpression param = Expression.Parameter(typeof(object[]), "args");

        Expression[] argsExp = new Expression[paramsInfo.Length];

        for (int i = 0; i < paramsInfo.Length; i++)
        {
            Expression index = Expression.Constant(i);
            Type paramType = paramsInfo[i].ParameterType;

            Expression paramAccessorExp = Expression.ArrayIndex(param, index);

            Expression paramCastExp = Expression.Convert(paramAccessorExp, paramType);

            argsExp[i] = paramCastExp;
        }

        NewExpression newExp = Expression.New(ctor, argsExp);

        LambdaExpression lambda = Expression.Lambda(typeof(ObjectActivator), newExp, param);

        return (ObjectActivator)lambda.Compile();
    }

    private ObjectActivator CreateActivator(string className)
    {
        Type type = Type.GetType(className);
        if (type == null)
            throw new ArgumentException("Incorrect class name", "className");

        // Get contructor with one parameter
        ConstructorInfo ctor = type.GetConstructors()
            .SingleOrDefault(w => w.GetParameters().Length == 1 
                && w.GetParameters()[0].ParameterType == typeof(object));

        if (ctor == null)
                throw new Exception("There is no any constructor with 1 object parameter.");

        return CreateActivator(ctor);
    }

    public ObjectActivator GetActivator(string className)
    {
        ObjectActivator activator;

        if (activators.TryGetValue(className, out activator))
        {
            return activator;
        }
        activator = CreateActivator(className);
        activators[className] = activator;
        return activator;
    }
}

The usage is following:

ActivatorsStorage ast = new ActivatorsStorage();
var a = ast.GetActivator("ConsoleApplication1.Operation1")(initializationData);
var b = ast.GetActivator("ConsoleApplication1.Operation2")(initializationData);

The same can be implemented with DynamicMethods.

Also, the classes are not required to be inherited from the same interface or base class.

Thanks, Vitaliy

Vitaliy
  • 2,744
  • 1
  • 24
  • 39
  • Are you saying with this code that you're actually using an Expression-builder for changing a constructor into a delegate, or are you creating a method that returns an initialized object (which is then stored in a delegate)? Quite impressive code, btw. – Abel Mar 10 '11 at 21:01
  • @Abel. Code is used to create delegate to the constructor with one parameter. For sure the functionality can be extended, added more verification, added interface usage, etc. – Vitaliy Mar 10 '11 at 21:08
  • that's not exactly what I meant... A constructor cannot be referenced by a delegate (because it allocates memory and returns a reference, also because it is _not_ a Method). So my guess is that the code above creates code that in fact wraps a call to the ctor. – Abel Mar 10 '11 at 21:20
  • @Abel, there is NewExpression class in the System.Linq.Expressions, which is used for constructing instances. So with Lambda it is created delegate, which calls the constructor – Vitaliy Mar 11 '11 at 08:15
6

One strategy that I use in cases like this is to flag my various implementations with a special attribute to indicate its key, and scan the active assemblies for types with that key:

[AttributeUsage(AttributeTargets.Class)]
public class OperationAttribute : System.Attribute
{ 
    public OperationAttribute(string opKey)
    {
        _opKey = opKey;
    }

    private string _opKey;
    public string OpKey {get {return _opKey;}}
}

[Operation("Standard deviation")]
public class StandardDeviation : IOperation
{
    public void Initialize(object originalData)
    {
        //...
    }
}

public interface IOperation
{
    void Initialize(object originalData);
}

public class OperationFactory
{
    static OperationFactory()
    {
        _opTypesByKey = 
            (from a in AppDomain.CurrentDomain.GetAssemblies()
             from t in a.GetTypes()
             let att = t.GetCustomAttributes(typeof(OperationAttribute), false).FirstOrDefault()
             where att != null
             select new { ((OperationAttribute)att).OpKey, t})
             .ToDictionary(e => e.OpKey, e => e.t);
    }
    private static IDictionary<string, Type> _opTypesByKey;
    public IOperation GetOperation(string opKey, object originalData)
    {
        var op = (IOperation)Activator.CreateInstance(_opTypesByKey[opKey]);
        op.Initialize(originalData);
        return op;
    }
}

That way, just by creating a new class with a new key string, you can automatically "plug in" to the factory, without having to modify the factory code at all.

You'll also notice that rather than depending on each implementation to provide a specific constructor, I've created an Initialize method on the interface I expect the classes to implement. As long as they implement the interface, I'll be able to send the "originalData" to them without any reflection weirdness.

I'd also suggest using a dependency injection framework like Ninject instead of using Activator.CreateInstance. That way, your operation implementations can use constructor injection for their various dependencies.

StriplingWarrior
  • 151,543
  • 27
  • 246
  • 315
  • +1, I like this strategy and actually, we use it in one of our larger projects involving automatic mapping. Using attributes can add what your originally loose: type safety. However, you still use the expensive `Activator.CreateInstance`, which returns a wrapper and not an instance of the original class (whether that matters to the OP is a different question). – Abel Mar 11 '11 at 11:07
  • @Abel: I believe you are mistaken: Activator.CreateInstance does create an instance of the original class, not a wrapper. – StriplingWarrior Mar 11 '11 at 17:43
  • 1
    you can see it for yourself: `object o = Activator.CreateInstance(null, "TestDynamicFactory.RandomNumber")` and then call `Debug.Write(o.GetType().ToString())`, you will see it prints "System.Runtime.Remoting.ObjectHandle", which is the actual type CreateInstance returns. It's intended use is for interop, not for calling the constructor. – Abel Mar 11 '11 at 18:23
  • 2
    @Abel: The overload I used in my code is `CreateInstance(Type)`, which calls the constructor. – StriplingWarrior Mar 11 '11 at 21:46
  • 1
    You are right, that overload works differently, apologies, I missed that detail. – Abel Mar 12 '11 at 11:44
1

Essentially, it sounds like you want the factory pattern. In this situation, you define a mapping of input to output types and then instantiate the type at runtime like you are doing.

Example:

You have X number of classes, and they all share a common interface of IDoSomething.

public interface IDoSomething
{
     void DoSomething();
}

public class Foo : IDoSomething
{
    public void DoSomething()
    {
         // Does Something specific to Foo
    }
}

public class Bar : IDoSomething
{
    public void DoSomething()
    {
        // Does something specific to Bar
    }
}

public class MyClassFactory
{
     private static Dictionary<string, Type> _mapping = new Dictionary<string, Type>();

     static MyClassFactory()
     {
          _mapping.Add("Foo", typeof(Foo));
          _mapping.Add("Bar", typeof(Bar));
     }

     public static void AddMapping(string query, Type concreteType)
     {
          // Omitting key checking code, etc. Basically, you can register new types at runtime as well.
          _mapping.Add(query, concreteType);
     }

     public IDoSomething GetMySomething(string desiredThing)
     {
          if(!_mapping.ContainsKey(desiredThing))
              throw new ApplicationException("No mapping is defined for: " + desiredThing);

          return Activator.CreateInstance(_mapping[desiredThing]) as IDoSomething;
     }
}
Tejs
  • 40,736
  • 10
  • 68
  • 86
1
  1. There's no error checking here. Are you absolutely sure that _class will resolve to a valid class? Are you controlling all the possible values or does this string somehow get populated by an end-user?
  2. Reflection is generally most costly than avoiding it. Performance issues are proportionate to the number of objects you plan to instantiate this way.
  3. Before you run off and use a dependency injection framework read the criticisms of it. =)
Suraj
  • 35,905
  • 47
  • 139
  • 250