Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have recently started writing C# from a strong JavaScript background, and found myself wanting to do something I do all the time in JavaScript: events.

Since I have a type system at my disposal, I figured I'd make use of it and make the event system type safe.

A quick note on existing options:

C#'s built-in event library didn't really appeal to me - for one, I don't really like the syntax (eg listeners += callback), but more than that, I wanted type-safety in the callbacks. (ie, I want the callback to be something like Action<T1, T2, ...>, not public void delegate SomeEventHandler(object sender, EventArgs args)).

Unity's built-in UnityEvent looks pretty good, but these aren't collected and managed in any way. I decided to roll my own Events to go with my EventManager, but I think I could adapt it to use UnityEvent fairly easily.

The code compiles and works, but I am wondering if there's a more elegant solution - or even if there's just a way I could reduce the boilerplate in my own solution. (Generics seem to cause most if not all of the boilerplate, but I didn't see a better way to get type-safety in callbacks.)

Here's how it is used:

using EventSystem;
// create SomeEvent - uses a class stub, not sure how I feel about that
public class SomeEvent : BasicEvent<SomeEvent, SomeParameterType> { }

// listen to it somewhere else in the codebase...
EventManager.On<SomeEvent, SomeParameterType>((param) => /* ... */);
// and trigger it somewhere different still...
EventManager.Trigger<SomeEvent, SomeParameterType>(someParameter);

Here's how it is structured:

  • namespace EventSystem
    • EventManager.cs, a static class that can be used to listen to / detach from and trigger events
    • EventBehaviour.cs, an abstract class that "registers" an event with the EventManager using Unity's Awake () { ... } - basically glue between the event system and Unity
    • IEventListener.cs, generic interfaces that define a type-safe listener, and have extension methods for implementing listener Invoke methods for any of the defined generics
    • BasicListener.cs, reusable implementations of each generic IEventListener
    • BasicEvent.cs, reusable containers deriving from EventBehaviour for each generic BasicListener

Here's a link to it on GitHub

And here are the files copy/pasted:

BasicEvent.cs

using System;
using System.Collections.Generic;

namespace EventSystem {
    public class BasicEvent<E> : EventBehaviour where E : EventBehaviour {
        public static BasicListener<E> listener = new BasicListener<E>();

        override public IEventListenerBase Listener {
            get {
                return listener;
            }
        }
    }

    public class BasicEvent<E, P> : EventBehaviour where E : EventBehaviour {
        public static BasicListener<E, P> listener = new BasicListener<E, P>();

        override public IEventListenerBase Listener {
            get {
                return listener;
            }
        }
    }

    public class BasicEvent<E, P1, P2> : EventBehaviour where E : EventBehaviour {
        public static BasicListener<E, P1, P2> listener = new BasicListener<E, P1, P2>();

        override public IEventListenerBase Listener {
            get {
                return listener;
            }
        }
    }
}

BasicListener.cs

using System;
using System.Collections.Generic;

namespace EventSystem {
    public class BasicListener<E> : IEventListener where E: EventBehaviour {
        private List<Action> _callbacks;
        private Type _eventType;

        public List<Action> Callbacks {
            get {
                return _callbacks;
            }
        }

        public Type EventType {
            get {
                return _eventType;
            }
        }

        public BasicListener () {
            _eventType = typeof(E);
            _callbacks = new List<Action>();
        }
    }

    public class BasicListener<E, P> : IEventListener<P> where E: EventBehaviour {
        private List<Action<P>> _callbacks;
        private Type _eventType;

        public List<Action<P>> Callbacks {
            get {
                return _callbacks;
            }
        }

        public Type EventType {
            get {
                return _eventType;
            }
        }

        public BasicListener () {
            _eventType = typeof(E);
            _callbacks = new List<Action<P>>();
        }
    }

    public class BasicListener<E, P1, P2> : IEventListener<P1, P2> where E: EventBehaviour {
        private List<Action<P1, P2>> _callbacks;
        private Type _eventType;

        public List<Action<P1, P2>> Callbacks {
            get {
                return _callbacks;
            }
        }

        public Type EventType {
            get {
                return _eventType;
            }
        }

        public BasicListener () {
            _eventType = typeof(E);
            _callbacks = new List<Action<P1, P2>>();
        }
    }
}

EventBehaviour.cs

using UnityEngine;
using System;
using System.Collections.Generic;

namespace EventSystem {
    public abstract class EventBehaviour : MonoBehaviour {
        abstract public IEventListenerBase Listener {
            get;
        }

        void Awake () {
            EventManager.Listeners.Add(this.Listener);
        }
    }
}

EventManager.cs

using System;
using System.Collections.Generic;

namespace EventSystem {
    public static class EventManager {
        public static List<IEventListenerBase> Listeners = new List<IEventListenerBase>();

        private static IEventListener GetListenerFor<E> () where E : EventBehaviour {
            Type eventType = typeof(E);
            return Listeners.Find((listener) => listener.EventType == eventType) as IEventListener;
        }

        private static IEventListener<P> GetListenerFor<E, P> () where E : EventBehaviour {
            List<IEventListener<P>> listeners;
            listeners = Listeners.FindAll((listener) => listener is IEventListener<P>)
                                 .ConvertAll<IEventListener<P>>((listener) => listener as IEventListener<P>);
            return listeners.Find((listener) => listener.Callbacks is List<Action<P>>);
        }

        private static IEventListener<P1, P2> GetListenerFor<E, P1, P2> () where E : EventBehaviour {
            List<IEventListener<P1, P2>> listeners;
            listeners = Listeners.FindAll((listener) => listener is IEventListener<P1, P2>)
                                 .ConvertAll<IEventListener<P1, P2>>((listener) => listener as IEventListener<P1, P2>);
            return listeners.Find((listener) => listener.Callbacks is List<Action<P1, P2>>);
        }

        public static void On<E> (Action callback) where E : EventBehaviour {
            IEventListener l = GetListenerFor<E>();
            l.Callbacks.Add(callback);
        }

        public static void On<E, P> (Action<P> callback) where E : EventBehaviour {
            IEventListener<P> l = GetListenerFor<E, P>();
            l.Callbacks.Add(callback);
        }

        public static void On<E, P1, P2> (Action<P1, P2> callback) where E : EventBehaviour {
            IEventListener<P1, P2> l = GetListenerFor<E, P1, P2>();
            l.Callbacks.Add(callback);
        }

        public static bool Off<E> (Action callback) where E : EventBehaviour {
            IEventListener l = GetListenerFor<E>();
            return l.Callbacks.Remove(callback);
        }

        public static bool Off<E, P> (Action<P> callback) where E : EventBehaviour {
            IEventListener<P> l = GetListenerFor<E, P>();
            return l.Callbacks.Remove(callback);
       }

        public static bool Off<E, P1, P2> (Action<P1, P2> callback) where E : EventBehaviour {
            IEventListener<P1, P2> l = GetListenerFor<E, P1, P2>();
            return l.Callbacks.Remove(callback);
       }

        public static void Trigger<E> () where E : EventBehaviour {
            IEventListener l = GetListenerFor<E>();
            l.Invoke();
        }

        public static void Trigger<E, P> (P arg) where E : EventBehaviour {
            IEventListener<P> l = GetListenerFor<E, P>();
            l.Invoke<P>(arg);
        }

        public static void Trigger<E, P1, P2> (P1 arg1, P2 arg2) where E : EventBehaviour {
            IEventListener<P1, P2> l = GetListenerFor<E, P1, P2>();
            l.Invoke<P1, P2>(arg1, arg2);
        }
    }
}

IEventListener.cs

using System;
using System.Collections.Generic;

namespace EventSystem {
    public interface IEventListenerBase {
        Type EventType { get; }
    }

    public interface IEventListener : IEventListenerBase {
        List<Action> Callbacks { get; }
    }

    public interface IEventListener<P1> : IEventListenerBase {
        List<Action<P1>> Callbacks { get; }
    }

    public interface IEventListener<P1, P2> : IEventListenerBase {
        List<Action<P1, P2>> Callbacks { get; }
    }

    public static class IEventListenerExtensions {
        public static void Invoke (this IEventListener l) {
            l.Callbacks.ForEach((cb) => cb());
        }

        public static void Invoke<P1> (this IEventListener<P1> l, P1 arg) {
            l.Callbacks.ForEach((cb) => cb(arg));
        }

        public static void Invoke<P1, P2> (this IEventListener<P1, P2> l, P1 arg1, P2 arg2) {
            l.Callbacks.ForEach((cb) => cb(arg1, arg2));
        }
    }
}
share|improve this question
    
Hm? I wrote all of the above code. I wrote the event system. – Jordan Feb 21 at 1:21
    
Oh! I'm very sorry, I completely misunderstood your post. Please excuse that comment. BTW, great job on your first question! – SirPython Feb 21 at 1:22

C# events are type-safe. The common (and recommended) pattern is to use EventHandler<T>, where T is a custom type deriving from EventArgs, containing the data of the event. You should use this pattern if possible, especially if you're writing a library that will be used by third parties.

The problems that I can see in this pattern are:

  1. The sender parameter is always object. It's either not useful (because you can access the object directly thanks to lambdas) or requires casting.
  2. The eventArgs parameter should derive from EventArgs. This means you need to litter your code with lots of types that derive from EventArgs, even if the data of the event is just a single value.

If you're set on not using this pattern, I think you should still use event, just not with EventHandler<T>.

The simplest version of that is just:

public event Action<SomeParameter> SomeEvent;
share|improve this answer
    
I think I had seen the doc page for EventHandler<T>, but was turned away by object sender. I would like to avoid casting. That said, using event with Action seems like it would make a lot of my code unnecessary, which is great haha. Thanks! – Jordan Feb 21 at 20:09

A few more problems I can see with your implementation:

  1. You'll need to add more boiler plate code when you want to have callbacks taking three arguments, then again more for four and for five, etc.

  2. The implementation isn't thread-safe and events are quite likely to be used across threads.

  3. Having to create marker classes for each event is a bit ugly and just adds more boiler plate code.

  4. It won't scale very very well if you have many different events, especially if they are called frequently, since you'll have to pay the price to search through the list every time. This could probably be mitigated by using something like a ConcurrentDictionary

  5. You could make better use of LINQ. This:

    listeners = Listeners.FindAll((listener) => listener is IEventListener<P1, P2>)
                         .ConvertAll<IEventListener<P1, P2>>((listener) => listener as IEventListener<P1, P2>);
    

    can be shortened to:

    listeners = Listeners.OfType<IEventListener<P1, P2>>();
    

So it's an interesting programming exercise but I wouldn't use it in production code.

share|improve this answer
    
When you say that I should use ConcurrentDictionary, does that also help address thread-safety, or just performance? Are built-in c# events thread-safe? – Jordan Feb 21 at 20:04
    
@Jordan: C# events are thread-safe in the sense that you can raise the event while some is un/subscribing from/to it. The ConcurrentDictionary would help with the central event listeners, you'd also have to put in a concurrent collection to store the actual callbacks – ChrisWue Feb 21 at 20:12
    
Since I'm using Unity3d, I can't use ConcurrentDictionary. Also, I'm not using threads in my game, so I guess it should be fine. As it turns out, Unity is largely not thread-safe to begin with. That's good to know, though! – Jordan Feb 22 at 6:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.