What do you think of my EventAggregator implementation in C#?

I was looking for a simple Event Aggregator to use in my app, but I found people get very complicated very quickly, so I thought this was easy enough to write myself. It seems to work, but I was wondering what you think and if you would change or add anything. (I know some of them raise events back on the same thread that it was registered, but for what I was planning I don't need this.)

public class EventAggregator
{
    private Dictionary> _listeners;

    private object _syncLock;

    public EventAggregator()
    {
        _listeners = new Dictionary>();
        _syncLock = new object();
    }

    public EventAggregator Register(Action eventAction)
    {
        LockAround(() =>
            {
                var listeners = GetListeners(typeof(TEvent));
                listeners.RemoveAll(wr => !wr.IsAlive || (Action)wr.Target == eventAction);
                listeners.Add(new WeakReference(eventAction));
            });
        return this;
    }

    public void Raise(TEvent ev)
    {
        List targets = null;
        LockAround(() =>
            {
                var listeners = GetListeners(typeof(TEvent));
                targets = listeners
                    .Where(wr => wr.IsAlive)
                    .ToList();
            });

        targets.ForEach(wr => ((Action)wr.Target)(ev));
    }

    private void LockAround(Action action)
    {
        lock (_syncLock)
        {
            action();
        }
    }

    private List GetListeners(Type type)
    {
        List result;
        if (_listeners.TryGetValue(type, out result))
        {
            return result;
        }
        result = new List();
        _listeners[type] = result;
        return result;
    }
}

This is how I plan to use it:

```
class Program
{
static void Main(string[] args)
{
var ea = new EventAggregator();

var view = new View(ea);
var p = new Presenter(ea);

view.Name = "Name1";
view.Name = "Name2";

Cons

I haven't thoroughly looked at it but these are my initial thoughts.

Minor points

I would initialise the private variables inline like this private object _syncLock = new Object(); because they don't depend on parameters given to the constructor.

For readability I wouldn't use the LockAround method, I would just lock _syncObject.

In GetListeners is there any reason you are adding a list to the dictionary when no events have been registered for that type? Also, you could make it generic by pulling in the typeof from Raise like so _listeners.TryGetValue(typeof(TEvent), out result).

In Register what is the purpose of (Action)wr.Target == eventAction? I think if someone wants to register the same event handler twice then they should be able to.

You might want to make your event aggregator a singleton or use a dependency injection framework to ensure there is only ever one instance in existence.

Worries

In Raise the following line worries me targets.ForEach(wr => ((Action)wr.Target)(ev));. I'm not sure there is any guarantee the weak reference will still be alive at this point. You probably want to solidify the reference and check that it is not null before calling it.

Finally

Have you looked at Microsoft's event aggregator and unity dependency injection framework? I think they probably do what you are looking for and will save you from reinventing the wheel.

StackExchange Code Review Q#5416, answer score: 5