2010-11-20 18 views
5

Me encontré con algún código el otro día y me pregunté si esa era la mejor manera de hacerlo. Tenemos un método que toma una cadena de algunos datos de formularios web que hace algo a un objeto en función de la cadena que se transmite. Actualmente, usa la reflexión para determinar qué acción tomar, pero me preguntaba si una instrucción de cambio sería mejor. .Método Factory - caso vs. reflexión

Ejemplo:

Edición: He añadido una tercera opción para los delegados como señaló 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); 
     } 
    } 

} 

El primer ejemplo se utiliza el interruptor y como se puede ver podría ser muy detallado. El segundo es mucho más corto, pero utiliza la reflexión, que sé que algunas personas evitan como la peste.

¿Funcionará un método significativamente mejor que el otro?

¿Cambiaría el rendimiento si hubiera 100 acciones diferentes en lugar de solo 3?

¿Qué prefieres ver en tu código si lo lees?

+0

Es más o menos una pregunta sobre el estilo de programación imperativa frente a declarativa. Kirk parece preferir el modo imperativo, mientras yo prefiero el último, como se puede ver en mi respuesta. Ninguno de los dos es "correcto" o "incorrecto", solo para aclarar esto. – Lucero

Respuesta

3

El primer caso casi siempre será más rápido. Sin embargo, su rendimiento proviene del hecho de que puede vincularse temprano durante el tiempo de compilación, pero ese es también su mayor inconveniente: este enfoque no puede, por ejemplo, manejar ensamblados dinámicamente cargados, y es mucho más propenso a errores ya que es imperativo y no declarativo. (Olvidar una acción recién implementada, por ejemplo, es algo que podría suceder rápidamente.)

Mi enfoque habitual es utilizar la reflexión para implementar dichos patrones durante el tiempo de descubrimiento, pero para usar delegados en el momento de la invocación. Esto le brinda la flexibilidad del enfoque de reflexión con un rendimiento muy cercano al enfoque de vinculación temprana.

  • fase de descubrimiento: utilizar la reflexión para encontrar los miembros (utilizando atributos, interfaces, firmas, y/o convenciones de codificación). En su caso, siempre tiene la misma firma, por lo que el delegado a usar sería Action<object>. Agregue esos miembros a una instancia Dictionary<string, Action<object>>, creando un delegado del MethodInfo usando CreateDelegate().

  • fase de Invocación: obtener el delegado través de su clave e invocar, lo cual es muy simple (en este caso suponiendo que el diccionario se llama methods): methods[action](o)

2

rendimiento no debería ser su preocupación a menos que perfilar y es un cuello de botella. Más importante, IMO, es que pierdes seguridad y análisis de tipo estático en la versión de reflexión. No hay forma de que en tiempo de compilación se compruebe si se invocan esos métodos de acción DoX, DOY, etc. Esto puede o no ser un problema para usted, pero esa sería mi mayor preocupación.

Además, el número de acciones es completamente irrelevante para el rendimiento de la versión de reflexión. GetMethod no se desacelera cuando tienes muchos miembros en tu clase.

+0

La invocación dinámica en un 'MethodInfo' es lenta, por lo que sería el mayor golpe de rendimiento. – Lucero

+0

@Lucero, claro, es más lento. La pregunta es si importa. En mi experiencia, muy raramente lo hace. –

2

¿qué tal el uso de delegados?usted podría utilizar algo así:

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

y más tarde

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

supongo que tiene un mejor rendimiento, si tiene muchos casos, debido a que el diccionario tiene una búsqueda de hash.

del tipo de reflexión que utilizó puede crear problemas de seguridad, si el usuario puede dar en otro nombre de la función

+0

Dudo que compile esto, dado el hecho de que 'DoX',' DoY' y 'DoZ' no son funciones, pero la lambda espera un valor de retorno. – Lucero

+0

bien corregido, "acción" es el tipo correcto para un delegado sin ningún valor de retorno – user287107

2

Puede fijar @ respuesta de user287107 así:

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

Esta es, en efecto, haciendo el descubrimiento fase de la respuesta de @ Lucero explícitamente, que puede ser útil si los nombres no siempre coinciden.

Definir el conjunto de acciones en una enumeración también sería bueno si fuera posible; esto sería un poco más rápido ya que no sería necesario manipular las cadenas. También le permitiría escribir una prueba unitaria para asegurarse de que no se haya perdido ningún valor potencial.