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
My question is: is this check necessary?
No, it is not.
why?
That's easy, the check is done by the operator that you're using. It was specifically designed to do nothing if the handler to be removed doesn't exist, rather than, say, throw an exception.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With