2008-10-09 20 views
8

Correr FxCop en mi código, me sale esta advertencia:El acoplamiento es demasiado alto: ¿cómo diseñar mejor esta clase?

Microsoft.Maintainability: 'FooBar.ctor se acopla con 99 tipos diferentes de 9 diferentes espacios de nombres. Reescriba o refactorice el método para disminuir su acoplamiento de clase, o considere mover el método a uno de los demás tipos, junto con . Un acoplamiento de clase superior a 40 indica un mantenimiento deficiente, un acoplamiento de clase entre 40 y 30 indica un mantenimiento moderado, y un acoplamiento de clase inferior a 30 indica una buena capacidad de mantenimiento.

Mi clase es una zona de aterrizaje para todos los mensajes del servidor. El servidor nos puede enviar mensajes de diferentes tipos: EventArgs

public FooBar() 
{ 
    var messageHandlers = new Dictionary<Type, Action<EventArgs>>(); 
    messageHandlers.Add(typeof(YouHaveBeenLoggedOutEventArgs), HandleSignOut); 
    messageHandlers.Add(typeof(TestConnectionEventArgs), HandleConnectionTest); 
    // ... etc for 90 other types 
} 

Los métodos "HandleSignOut" y "HandleConnectionTest" tienen poco código en ellos; por lo general, pasan el trabajo a una función en otra clase.

¿Cómo puedo mejorar esta clase con un acoplamiento más bajo?

+0

Me gustan las respuestas aquí, pero tal vez en algún lugar como RefactorMyCode.com podría ser un mejor lugar para ello. Es un poco más fácil para todos publicar código en sus respuestas en ese sitio. –

+0

Ok, publiqué el ejemplo, compruébalo –

Respuesta

15

Tener las clases que hacen el registro de trabajo para eventos en los que están interesados ​​... un patrón event broker.

class EventBroker { 
    private Dictionary<Type, Action<EventArgs>> messageHandlers; 

    void Register<T>(Action<EventArgs> subscriber) where T:EventArgs { 
     // may have to combine delegates if more than 1 listener 
     messageHandlers[typeof(T)] = subscriber; 
    } 

    void Send<T>(T e) where T:EventArgs { 
     var d = messageHandlers[typeof(T)]; 
     if (d != null) { 
     d(e); 
     } 
    } 
} 
+0

Exactamente de lo que estaba hablando. Estoy votando el tuyo por el ejemplo. – NotMe

+0

¡Interesante! Gracias por la respuesta, tomaremos esto en consideración. Creo que aceptaré la tuya como respuesta hasta que vea una mejor. –

+0

¡Interesante! Me encontré con el mismo problema. ¿Es lo suficientemente rápido? Quiero decir, ¿puedo usarlo en un juego con un ciclo de actualización? – Vinz243

0

Quizás en lugar de tener una clase diferente para cada mensaje, use una bandera que identifique el mensaje.

Eso reduciría drásticamente la cantidad de mensajes que tiene y aumentaría la capacidad de mantenimiento. Creo que la mayoría de las clases de mensajes tienen una diferencia de cero.

Es difícil elegir una forma adicional de atacar esto porque el resto de la arquitectura es desconocida (para mí).

Si mira a Windows, por ejemplo, no sabe de forma nativa cómo manejar cada mensaje que pueda surgir. En cambio, los manejadores de mensajes subyacentes registran las funciones de devolución de llamada con el hilo principal.

Puede tener un enfoque similar. Cada clase de mensaje necesitaría saber cómo manejarse a sí misma y podría registrarse con la aplicación más grande. Esto debería simplificar en gran medida el código y deshacerse del acoplamiento ajustado.

+0

Gracias por la respuesta. Probamos esto hace algún tiempo, pero simplemente cambia la complejidad de las funciones del controlador: en lugar de 99 funciones de controlador pequeñas, tiene 20 funciones de controlador muy grandes. Alguna mejor idea? –

5

También podría usar algún tipo de marco de IoC, como Spring.NET, para inyectar el diccionario. De esta forma, si obtiene un nuevo tipo de mensaje, no tiene que volver a compilar este concentrador central, solo cambie un archivo de configuración.


El ejemplo tan esperado:

Crear una nueva aplicación de consola, llamada ejemplo, y añadir lo siguiente:

using System; 
using System.Collections.Generic; 
using Spring.Context.Support; 

namespace Example 
{ 
    internal class Program 
    { 
     private static void Main(string[] args) 
     { 
      MessageBroker broker = (MessageBroker) ContextRegistry.GetContext()["messageBroker"]; 
      broker.Dispatch(null, new Type1EventArgs()); 
      broker.Dispatch(null, new Type2EventArgs()); 
      broker.Dispatch(null, new EventArgs()); 
     } 
    } 

    public class MessageBroker 
    { 
     private Dictionary<Type, object> handlers; 

     public Dictionary<Type, object> Handlers 
     { 
      get { return handlers; } 
      set { handlers = value; } 
     } 

     public void Dispatch<T>(object sender, T e) where T : EventArgs 
     { 
      object entry; 
      if (Handlers.TryGetValue(e.GetType(), out entry)) 
      { 
       MessageHandler<T> handler = entry as MessageHandler<T>; 
       if (handler != null) 
       { 
        handler.HandleMessage(sender, e); 
       } 
       else 
       { 
        //I'd log an error here 
        Console.WriteLine("The handler defined for event type '" + e.GetType().Name + "' doesn't implement the correct interface!"); 
       } 
      } 
      else 
      { 
       //I'd log a warning here 
       Console.WriteLine("No handler defined for event type: " + e.GetType().Name); 
      } 
     } 
    } 

    public interface MessageHandler<T> where T : EventArgs 
    { 
     void HandleMessage(object sender, T message); 
    } 

    public class Type1MessageHandler : MessageHandler<Type1EventArgs> 
    { 
     public void HandleMessage(object sender, Type1EventArgs args) 
     { 
      Console.WriteLine("Type 1, " + args.ToString()); 
     } 
    } 

    public class Type2MessageHandler : MessageHandler<Type2EventArgs> 
    { 
     public void HandleMessage(object sender, Type2EventArgs args) 
     { 
      Console.WriteLine("Type 2, " + args.ToString()); 
     } 
    } 

    public class Type1EventArgs : EventArgs {} 

    public class Type2EventArgs : EventArgs {} 
} 

y un archivo app.config:

<?xml version="1.0" encoding="utf-8" ?> 
<configuration> 
    <configSections> 
    <sectionGroup name="spring"> 
     <section name="context" type="Spring.Context.Support.ContextHandler, Spring.Core"/> 
     <section name="objects" type="Spring.Context.Support.DefaultSectionHandler, Spring.Core"/> 
    </sectionGroup> 
    </configSections> 

    <spring> 
    <context> 
     <resource uri="config://spring/objects"/> 
    </context> 
    <objects xmlns="http://www.springframework.net"> 

     <object id="messageBroker" type="Example.MessageBroker, Example"> 
     <property name="handlers"> 
      <dictionary key-type="System.Type" value-type="object"> 
      <entry key="Example.Type1EventArgs, Example" value-ref="type1Handler"/> 
      <entry key="Example.Type2EventArgs, Example" value-ref="type2Handler"/> 
      </dictionary> 
     </property> 
     </object> 
     <object id="type1Handler" type="Example.Type1MessageHandler, Example"/> 
     <object id="type2Handler" type="Example.Type2MessageHandler, Example"/> 
    </objects> 
    </spring> 
</configuration> 

Salida:

 
Type 1, Example.Type1EventArgs 
Type 2, Example.Type2EventArgs 
No handler defined for event type: EventArgs 

Como puede ver, MessageBroker no tiene conocimiento de ninguno de los controladores, y los controladores no saben acerca de MessageBroker. Todo el mapeo se hace en la aplicación.config, de modo que si necesita manejar un nuevo tipo de evento, puede agregarlo en el archivo de configuración. Esto es especialmente bueno si otros equipos están definiendo tipos de eventos y manejadores: pueden simplemente compilar sus cosas en un dll, lo dejan caer en su implementación y simplemente agregan un mapeo.

El diccionario tiene valores de tipo objeto en lugar de MessageHandler<> porque los controladores reales no se pueden convertir a MessageHandler<EventArgs>, así que tuve que hackear un poco ese aspecto. Creo que la solución aún está limpia y maneja bien los errores de mapeo. Tenga en cuenta que también deberá hacer referencia a Spring.Core.dll en este proyecto. Puede encontrar las bibliotecas here y la documentación here. El es relevante para esto. También tenga en cuenta que no hay ninguna razón por la que necesite usar Spring.NET para esto; la idea importante aquí es la inyección de dependencia. De alguna manera, algo va a necesitar decirle al agente que envíe mensajes de tipo a a x, y usar un contenedor IoC para la inyección de dependencia es una buena manera de que el agente no sepa sobre x, y viceversa.

Algunos otros SO pregunta relacionada con la COI y DI:

+0

Gracias por la respuesta. Genial, eso suena interesante.No estoy claro en un aspecto: ¿el marco de trabajo de IoC aún puede usar mis funciones de controlador? Quiero decir, mi diccionario está compuesto por las claves EventArg y los valores de la función del manejador. Ioc trabaja con los valores de la función del controlador todavía? –

+0

Los manejadores tendrían que dividirse en clases separadas que implementaran una interfaz, y luego llamarían a su método "handleMessage". O bien, podría pasar cadenas y usar la reflexión para captar los métodos, aunque eso es un tipo de mal olor de código. Puedo editar con un ejemplo si quieres. –

+0

Ok, entonces en lugar de saber de 99 tipos de FooBar, tendríamos 99 tipos que conocen FooBar. Me encantaría ver una muestra. –

0

no veo el resto de su código, pero me gustaría probar crea un número mucho más pequeño de clases de eventos Arg. En su lugar, cree unos pocos que sean similares entre sí en términos de datos contenidos y/o la forma en que los maneja más adelante y agregue un campo que le dirá qué tipo exacto de evento ocurrió (probablemente debería usar una enumeración).

Lo ideal es que no sólo haría que este constructor mucho más fácil de leer, sino también la forma en que los mensajes son manejados (mensajes de grupo que se manejan de una manera similar en un solo controlador de eventos)

+0

Gracias por la respuesta. Chris Lively sugirió la menor cantidad de clases de eventos arg. Probamos eso, y simplemente cambia la complejidad: en lugar de 99 funciones de controlador pequeñas, ahora tenemos 20 funciones de controlador grande. –

+0

En cuanto a los "mensajes de grupo que se manejan de manera similar en un único controlador de eventos", sí, lo hacemos. Tenemos alrededor de 20 eventos args que se manejan de la misma manera y pasan al mismo controlador. –

0

Obviamente usted está en necesidad de un mecanismo de reenvío: dependiendo del evento que recibe, que desea ejecutar código diferente.

Parece que está utilizando el sistema de tipos para identificar los eventos, mientras que en realidad está destinado a apoyar el polimorfismo. Como sugiere Chris Lively, también podría (sin abusar del sistema de tipos) usar una enumeración para identificar los mensajes.

O puede aprovechar la potencia del sistema de tipos, y crear un objeto de registro donde se registran todos los tipos de eventos (por una instancia estática, un archivo de configuración o lo que sea). Luego, podría usar el patrón Cadena de responsabilidad para encontrar el controlador adecuado. O bien el manejador hace el manejo en sí mismo, o puede ser una fábrica, creando un objeto que maneja el evento.

Este último método parece poco especificado y sobredimensionado, pero en el caso de 99 tipos de eventos (ya), me parece apropiado.

+0

No entiendo su sugerencia. ¿Puede indicarme más recursos o publicar un ejemplo? –

Cuestiones relacionadas