Is it necessary to check if a handler exists in a delegate chain before removing it?

Viewed 2354

I have a small snippet of a delegate-based messaging system, you could Subscribe and Unsubscribe event handlers, and Raise new events.

In my Unsubscribe method, I check to make sure that the handler exist first, before removing it.

My question is: is this check necessary? i.e. can't I just do:

dic[type] -= handler;

without:

if (handler exists)
    dic[type] -= handler;

if so, why? - I tried both checking and not checking, they both yield the same practical results. Not sure if there's any reasons why I'd prefer to use either.

Code:

    public abstract class GameEvent { }

    private static class EventManagerInternal<T> where T : GameEvent
    {
        private static Dictionary<Type, Action<T>> dic = new Dictionary<Type, Action<T>>();

        public static void Subscribe(Action<T> handler)
        {
            Type type = typeof(T);
            if (!dic.ContainsKey(type)) {
                dic[type] = handler;
                Console.WriteLine("Registered new type: " + type);
            }
            else {
                // make sure the handler doesn't exist first
                bool hasHandlerSubscribed = dic[type].GetInvocationList().Any(h => h.Equals(handler));
                if (hasHandlerSubscribed) {
                    Console.WriteLine(handler.Method.Name + " has already subbed to an event of type " + type);
                    return;
                }
                dic[type] += handler;
            }
            Console.WriteLine("Method " + handler.Method.Name + " has subbed to receive notifications from " + type);
        }

        public static void Unsubscribe(Action<T> handler)
        {
            Type type = typeof(T);

            // make sure the type exists
            if (!dic.ContainsKey(type)) {
                Console.WriteLine("Type " + type + " hasn't registered at all, it doesn't have any subscribers... at least not in my book...");
                return;
            }

            // get the methods that the delegate points to
            // to see if the handler exists or not
            bool exist = dic[type].GetInvocationList().Any(h => h.Equals(handler));
            if (!exist) {
                Console.WriteLine("Method " + handler.Method.Name + " hasn't registered at all, for notifications from " + type);
                return;
            }

            // remove the handler from the chain
            dic[type] -= handler;
            Console.WriteLine("handler " + handler.Method.Name + " has been removed. it won't take any notifications from " + type);

            // if there's no more subscribers to the "type" entry, remove it
            if (dic[type] == null) {
                dic.Remove(type);
                Console.WriteLine("No more subscribers to " + type);
            }
        }

        public static void Raise(T e)
        {
            Action<T> handler;
            if (dic.TryGetValue(e.GetType(), out handler)) {
                handler.Invoke(e);
            }
        }
    }

Example Usage: (via a wrapper - I just thought that Class.DoSomething<T> is nicer than Class<T>DoSomething - There is a good reason why I had to go for this setup :) Not the point here though...)

        Player p = new Player();
        EventManager.Subscribe<OnRename>(OnRenameHandler);
        EventManager.Subscribe<OnRename>(AnotherOnRenameHandler);
        EventManager.Raise(new OnRename());
        EventManager.Unsubscribe<OnRename>(OnRenameHandler);
        etc
1 Answers
Related