Coupling is too high - how to design this class better?

Question

Running FxCop on my code, I get this warning:

Microsoft.Maintainability : 'FooBar.ctor is coupled with 99 different types from 9 different namespaces. Rewrite or refactor the method to decrease its class coupling, or consider moving the method to one of the other types it is tightly coupled with. A class coupling above 40 indicates poor maintainability, a class coupling between 40 and 30 indicates moderate maintainability, and a class coupling below 30 indicates good maintainability.

My class is a landing zone for all messages from the server. The server can send us messages of different EventArgs types:

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

The "HandleSignOut" and "HandleConnectionTest" methods have little code in them; they usually pass the work off to a function in another class.

How can I make this class better with lower coupling?

Answer 1

Perhaps instead of having a different class for each message, use a flag that identifies the message.

That would drastically reduce the number of messages you have and increase maintainability. My guess is that most of the message classes have about zero difference.

It's hard to pick an additional way of attacking this because the rest of the architecture is unknown (to me).

If you look at Windows, for example, it doesn't natively know how to handle each message that might be thrown about. Instead, the underlying message handlers register callback functions with the main thread.

You might take a similiar approach. Each message class would need to know how to handle itself and could register itself with the larger application. This should greatly simplify the code and get rid of the tight coupling.

Answer 2

You could also use some sort of IoC framework, like Spring.NET, to inject the dictionary. This way, if you get a new message type, you don't have to recompile this central hub - just change a config file.

---

The long awaited example:

Create a new console app, named Example, and add this:

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 {}
}

And an app.config file:

<?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"&gt;

      <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>

Output:

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

As you can see, MessageBroker doesn't know about any of the handlers, and the handlers don't know about MessageBroker. All of the mapping is done in the app.config file, so that if you need to handle a new event type, you can add it in the config file. This is especially nice if other teams are defining event types and handlers - they can just compile their stuff in a dll, you drop it into your deployment and simply add a mapping.

The Dictionary has values of type object instead of MessageHandler<> because the actual handlers can't be cast to MessageHandler<EventArgs>, so I had to hack around that a bit. I think the solution is still clean, and it handles mapping errors well. Note that you'll also need to reference Spring.Core.dll in this project. You can find the libraries here, and the documentation here. The dependency injection chapter is relevant to this. Also note, there is no reason you need to use Spring.NET for this - the important idea here is dependency injection. Somehow, something is going to need to tell the broker to send messages of type a to x, and using an IoC container for dependency injection is a good way to have the broker not know about x, and vice versa.

Some other SO question related to IoC and DI:

• Difference between Dependency Injection (DI) & Inversion of Control (IOC)
• Which C#/.net Dependency Injection frameworks are worth looking into?
• Which Dependency Injection Tool Should I Use?

Answer 3

I don't see the rest of your code, but I would try create a much smaller number of Event arg classes. Instead create a few that are similar to each other in terms of data contained and/or the way you handle them later and add a field that will tell you what exact type of event occured (probably you should use an enum).

Ideally you would not only make this constructor much more readable, but also the way the messages are handled (group messages that are handled in a similar way in a single event handler)

Answer 4

Have the classes that do the work register for events they're interested in...an event broker pattern.

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);
      }
   }
}

Answer 5

Obviously you are in need of a dispatching mechanism: depending on the event you receive, you want to execute different code.

You seem to be using the type system to identify the events, while it's actually meant to support polymorphism. As Chris Lively suggests, you could just as well (without abusing the type system) use an enumerate to identify the messages.

Or you can embrace the power of the type system, and create a Registry object where every type of event is registered (by a static instance, a config file or whatsoever). Then you could use the Chain of Responsibility pattern to find the proper handler. Either the handler does the handling itself, or it may be a Factory, creating an object that handles the event.

The latter method looks a bit underspecified and overengineered, but in the case of 99 event types (already), it seems appropriate to me.