Method Factory - case vs. reflection

问题:

I came across some code the other day and I wondered if that was the best way to do it. We have a method that takes a string from some web form data a does something to an object based on the string that is passed in. Currently, it uses reflection to figure which action to take, but I wondered if a switch statement would be better.

Example:

Edit: I added a third option for delegates as noted by Lucerno

public class ObjectManipulator
{
    private void DoX(object o) { }
    private void DoY(object o) { }
    private void DoZ(object o) { }

    public void DoAction(string action, object o)
    {
        switch (action)
        {
            case "DoX":
                DoX(o);
                break;
            case "DoY":
                DoY(o);
                break;
            case "DoZ":
                DoZ(o);
                break;
            default:
                throw new Exception(string.Format(
                    "Cannot locate action:{0}", action));
        }
    }

    public void DoActionViaReflection(string action, object o)
    {
        MethodInfo method = typeof(ObjectManipulator).
            GetMethod(action, new Type[] { typeof(object) });
        if (method == null)
        {
            throw new Exception(string.Format(
                "Cannot locate action:{0}", action));
        }
        else
        {
            method.Invoke(this, new object[] { o });
        }
    }
    private Dictionary<string, Action<object>> _methods;
    public ObjectManipulator()
    {
        _methods = new Dictionary<string, Action<object>>()
        {
            {"DoX", o => DoX(o)},
            {"DoY", o => DoY(o)},
            {"DoZ", o => DoZ(o)}
        };
    }
    public void DoActionViaDelegates(string action, object o)
    {
        if (!_methods.ContainsKey(action))
        {
            throw new Exception(string.Format(
                "Cannot locate action:{0}", action));
        }
        else
        {
            _methods[action](o);
        }
    }

}

The first example uses the switch and as you can see could get very verbose. The second is much shorter, but uses reflection, which I know some people avoid like the plague.

Will one method perform significantly better that the other?

Would the performance change if there were 100 different actions instead of just 3?

Which do you rather see in your code if you were reading it?

回答1:

The first case will almost always be faster. However, its performance comes from the fact that it can be early bound during compile time, but that is its biggest drawback as well: this approach cannot, for instance, handle dynamically loaded assemblies, and it is much more error-prone since it is imperative and not declarative. (Forgetting a newly implemented action for instance is a thing which could happen quickly.)

My usual approach is to use reflection to implement such patterns during discovery time, but to use delegates at invocation time. This gets you the flexibility of the reflection approach with performance very close to the early-bound approach.

• Discovery phase: use reflection to find the members (using attributes, interfaces, signatures, and/or coding conventions). In your case you always have the same signature, so the delegate to use would be Action<object>. Add those members to a Dictionary<string, Action<object>> instance, creating a delegate from the MethodInfo using CreateDelegate().

• Invocation phase: get the delegate via its key and invoke it, which is very simple (here assuming the dictionary is called methods): methods[action](o)

回答2:

Performance shouldn't be your concern unless you profile it and it's a bottleneck. More significant, IMO, is that you lose static type safety and analysis in the reflection version. There's no way at compile-time to check whether those action methods DoX, DOY, etc. are being invoked. This may or may not be an issue for you, but that would be my biggest concern.

Also, the number of actions is completely irrelevent for performance of the reflection version. GetMethod does not slow down when you have lots of members in your class.

回答3:

You can fix @user287107's answer thusly:

var Actions = new Dictionary<String, Action<object>>();
Actions["DoX"] = DoX;
Actions["DoY"] = DoY;
Actions["DoZ"] = DoZ;

This is in effect doing the discovery phase of @Lucero's answer explicitly, which may be useful if the names don't always match.

Defining the set of actions in an enum would also be nice if possible - this would be a tiny bit faster as you wouldn't need to hash the strings. It would also enable you to write a unit test to make sure you've not missed any potential values.

回答4:

how about using delegates? you could use something like that:

var Actions = new Dictionary<String, Action<object>>();
Actions["DoX"] = x => DoX(x); 
Actions["DoY"] = x => DoY(x); 
Actions["DoZ"] = x => DoZ(x); 

and later

public void DoAction(string action, object o)
{
    Actions[action](o);
}

i guess that performs better, if you have many cases, because the dictionary has a hash lookup.

the type of reflection you used can create security issues, if the user can give in another function name