Reputation: 707
I was looking through some old code today and found an event handler that looked like this:
public void HandleEvent(EventClassA eventObj)
{
if(eventObj is EventSubClassA)
{
HandleEventSubClassA(eventObj as EventSubClassA);
}
else if(eventObj is EventSubClassB)
{
HandleEventSubClassB(eventObj as EventSubClassB);
}
else if(eventObj.GetType() == typeof(EventSubClassC))
{
HandleEventSubClassC(eventObj as EventSubClassC);
}
else if(eventObj is EventSubClassD)
{
HandleEventSubClassD(eventObj as EventSubClassD);
}
}
I thought this was kind of ugly. So I refactored it like this:
delegate void EventHandler(dynamic eventObj);
private static readonly Dictionary<Type, EventHandler> EVENT_MAP = new Dictionary<Type, EventHandler>()
{
{ typeof(EventSubClassA), HandleEventSubClassA },
{ typeof(EventSubClassB), HandleEventSubClassB },
{ typeof(EventSubClassC), HandleEventSubClassC },
{ typeof(EventSubClassD), HandleEventSubClassD }
};
public void HandleEvent(EventClassA eventObj)
{
EVENT_MAP[eventObj.GetType()](eventObj);
}
private void HandleEventSubClassA(dynamic evt)
{
var eventObj = evt as EventSubClassA;
}
I had a coworker review the code and there were concerns about the way this solution worked compared to the previous solution. I have a hard time believing that the previous solution is the best solution for this case, so I've turned to StackOverflow.
Is there a better way to build this type of class? Is there a pattern I'm not aware of that is designed for this?
Upvotes: 4
Views: 1164
Reputation: 144136
You can use generics to make your existing solution slightly safer:
private static Dictionary<Type, Delegate> handlers;
static HandlerClass()
{
handlers = new Dictionary<Type, Delegate>();
AddHandler<EventSubClassA>(HandleEventSubClassA);
AddHandler<EventSubClassB>(HandleEventSubClassB);
...
}
public static void AddHandler<T>(Action<T> handler) where T : EventClassA
{
handlers[typeof(T)] = handler;
}
public void HandleEvent(EventClassA @event)
{
Delegate handler;
if(handlers.TryGetValue(@event.GetType(), out handler))
{
handler.DynamicInvoke(@event);
}
}
Alternatively, if you can modify the classes in your event hierarchy you could implement the visitor pattern:
public interface IHandlers
{
void HandleSubClassA(EventSubClassA a);
void HandleSubClassB(EventSubClassB b);
...
}
public abstract class EventClassA
{
public abstract void Visit(IHandlers handlers);
}
public class EventSubClassA : EventClassA
{
public override void Visit(IHandlers handlers)
{
handlers.HandleSubClassA(this);
}
}
Upvotes: 3
Reputation: 120460
I feel like I'm missing something. Wouldn't the best way to be to write overloads for each event type?
Upvotes: 1