Making an Action<T> event also usable as Action<object> and Action in C#

Question

Edit

I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659

---

Context

Before presenting the code for review, I want to be clear that I am not looking for recommendations on how to avoid having to solve the issue that the code is trying to solve in the first place

This is not production code, this is purely for exploratory purposes, and I would prefer answers that remain within the provided requirements.

With that said, let's suppose I have this contrived interface:

public interface IFoo<T>
{
    event Action<T> ChangeValue;
    event Action<object> ChangeObject;
    event Action ChangeEmpty;

    void InvokeChange(T value);
}

With the following contrived requirements:

• Calling InvokeChange with a value of type T that Foo was constructed with will invoke the three events

  • ChangeValue is invoked with the given value as type T
  • ChangeObject is invoked with the given value as type object
  • ChangeEmpty is invoked with no arguments

• Handlers that listen to any of the three events are invoked in the global order they were added, regardless of what event they were added to.

  • Effectively there is only a single event here - some 'change' happened - but 3 event signatures that can be utilized by listeners as needed.

• The implementation can take any form, but must implement IFoo<T> as it is specified above.

• The implementation does not need to be thread safe and can allocate memory, but no memory should leak when it is used correctly.

Example 1

The order that event listeners are called should correspond to the order in which they were added, regardless of which event they were added to:

IFoo<int> test = new Foo<int>();
test.ChangeEmpty  += ()  => Console.WriteLine("0) Empty!");
test.ChangeObject += (o) => Console.WriteLine("1) Object: {0}", o);
test.ChangeValue  += (v) => Console.WriteLine("2) Value:  {0}", v);
test.ChangeObject += (o) => Console.WriteLine("3) Object: {0}", o);
test.ChangeEmpty  += ()  => Console.WriteLine("4) Empty!");
test.InvokeChange(123);

Should print the following to the Console:

0) Empty!
1) Object: 123
2) Value:  123
3) Object: 123
4) Empty!

Example 2

Adding/Removing event Listeners should work as expected, while still maintaining the same order:

public static void Main()
{
    IFoo<int> test = new Foo<int>();

    test.ChangeEmpty += EmptyHandler;
    test.ChangeObject += ObjectHandler;
    Console.WriteLine("1) EMPTY, OBJECT");
    test.InvokeChange(1);

    test.ChangeEmpty -= EmptyHandler;
    test.ChangeValue += ValueHandler;
    Console.WriteLine("2) OBJECT, VALUE");
    test.InvokeChange(2);

    test.ChangeObject -= ObjectHandler;
    Console.WriteLine("3) VALUE ");
    test.InvokeChange(3);

    test.ChangeObject += ObjectHandler;
    test.ChangeEmpty += EmptyHandler;
    test.ChangeValue += ValueHandler;
    Console.WriteLine("4) VALUE, OBJECT, EMPTY, VALUE");
    test.InvokeChange(4);

    test.ChangeValue -= ValueHandler;
    test.ChangeValue -= ValueHandler;
    test.ChangeEmpty -= EmptyHandler;
    test.ChangeObject -= ObjectHandler;
    Console.WriteLine("5) <NONE>");
    test.InvokeChange(5);
}

static void EmptyHandler() { Console.WriteLine("  - Empty!"); }
static void ObjectHandler(object val) { Console.WriteLine("  - Object: {0}", val); }
static void ValueHandler(int val) { Console.WriteLine("  - Value: {0}", val); }

Should print the following to the Console:

1) EMPTY, OBJECT
  - Empty!
  - Object: 1
2) OBJECT, VALUE
  - Object: 2
  - Value: 2
3) VALUE 
  - Value: 3
4) VALUE, OBJECT, EMPTY, VALUE
  - Value: 4
  - Object: 4
  - Empty!
  - Value: 4
5) <NONE>

Effectively it should perform as if there is a single invokation list that backs all 3 events.

This code is not required to be particularly performant or thread safe, but 'correct'. In other words, invocation order and frequency is correct regardless of how many times a given handler is added/removed from one of the three events.

---

Code For Review

With all that in mind, the solution I came up with is:

• ChangeValue is a standard event backed by an Action<T>
• ChangeObject and ChangeEmpty are custom event accessors that provide a custom add and remove implementation which are ultimately also backed by ChangeValue.
  • The value of ChangeObject.add and ChangeEmpty.add is wrapped in a lambda 'proxy' of signature Action<T>
  • To ensure that the event accessors are able to also remove the corresponding proxy when in ChangeObject.remove and ChangeEmpty.remove, a dictionary is used for each to track a stack of proxies that have been bound for each value added.
    • add pushes a proxy to the stack and is then added with ChangeValue += proxy
    • remove pops a proxy from the stack and is then removed with ChangeValue -= proxy

Here is the relevant code for one of the events:

public event Action<T> ChangeValue = delegate {};

private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
    = new Dictionary<Action<object>, List<Action<T>>>();

public event Action<object> ChangeObject
{
    add
    {
        List<Action<T>> eventProxies;
        if(!ObjectEventProxies.TryGetValue(value, out eventProxies))
        {
            eventProxies = new List<Action<T>>();
            ObjectEventProxies.Add(value, eventProxies);
        }
        Action<T> proxy = (T v) => value.Invoke(v);
        eventProxies.Add(proxy);
        ChangeValue += proxy;
    }
    remove
    {
        List<Action<T>> eventProxies;
        if (ObjectEventProxies.TryGetValue(value, out eventProxies))
        {
            Action<T> proxy = eventProxies[eventProxies.Count - 1];
            eventProxies.RemoveAt(eventProxies.Count - 1);
            ChangeValue -= proxy;
        }
    }
}

And the full code, including the above examples: https://dotnetfiddle.net/u7NGMJ

Question

This code fulfills the given requirements but my question is, can this be simplified? There's a fair bit of boilerplate here for something that feels like it should be simpler to do.

---

To re-iterate, I am looking specifically for answers that still meet all of the listed requirements, regardless of how contrived or unnecessary they might seem. I should be able to swap out my IFoo<T> implementation for yours and have the output be identical.

I am aware I could just use a single Action<object> if I wanted a single signature to work for all cases. That is not what this question is asking.

---

Edit

I've added a second pass at this problem based on the answers that have been posted by other users so far: https://codereview.stackexchange.com/a/221525/75659

Answer 1

IMO your events should accommodate the C# standard for events:

  public interface IFoo<T>
  {
    event EventHandler<T> ChangeValue;
    event EventHandler<object> ChangeObject;
    event EventHandler ChangeEmpty;

    void InvokeChange(T value);
  }

The EventHandler delegate takes a source object as argument, which may be useful when consuming the event.

---

You can modify dfhwze's solution slightly to meet all your requirements to:

  public class Foo<T> : IFoo<T>
  {

    private List<object> handlers = new List<object>();

    void AddHandler(object handler)
    {
      handlers.Add(handler);
    }

    void RemoveHandler(object handler)
    {
      int index = handlers.LastIndexOf(handler);
      if (index >= 0)
        handlers.RemoveAt(index);
    }

    public event EventHandler<T> ChangeValue { add => AddHandler(value); remove => RemoveHandler(value); }
    public event EventHandler<object> ChangeObject { add => AddHandler(value); remove => RemoveHandler(value); }
    public event EventHandler ChangeEmpty { add => AddHandler(value); remove => RemoveHandler(value); }

    public void InvokeChange(T value)
    {
      foreach (object handler in handlers)
      {
        Invoke((dynamic)handler, value);
      }
    }

    public void Invoke(EventHandler handler, T value)
    {
      handler?.Invoke(this, EventArgs.Empty);
    }

    public void Invoke<S>(EventHandler<S> handler, S value)
    {
      handler?.Invoke(this, value);
    }
  }

A List<object> may not be the most effective container, if you have many listeners. In that case you'll have to find another.

---

Another solution building on your own could be:

  public class Foo<T> : IFoo<T>
  {
    private event EventHandler<T> AllEvents;
    private Dictionary<Delegate, (EventHandler<T> Handler, int Count)> allHandlers = new Dictionary<Delegate, (EventHandler<T>, int)>();

    void AddHandler(Delegate source, Func<EventHandler<T>> handlerFactory)
    {
      if (!allHandlers.TryGetValue(source, out var handler))
      {
        handler = (handlerFactory(), 1);
      }
      else
      {
        handler.Count++;
      }

      allHandlers[source] = handler;
      AllEvents += handler.Handler;
    }

    void RemoveHandler(Delegate source)
    {
      if (allHandlers.TryGetValue(source, out var handler))
      {
        handler.Count--;
        AllEvents -= handler.Handler;
        if (handler.Count <= 0)
          allHandlers.Remove(source);
        else
          allHandlers[source] = handler;
      }
    }

    public event EventHandler<T> ChangeValue { add => AddHandler(value, () => value); remove => RemoveHandler(value); }
    public event EventHandler<object> ChangeObject { add => AddHandler(value, () => (src, v) => value.Invoke(src, v)); remove => RemoveHandler(value); }
    public event EventHandler ChangeEmpty { add => AddHandler(value, () => (src, v) => value.Invoke(src, EventArgs.Empty)); remove => RemoveHandler(value); }

    public void InvokeChange(T value)
    {
      AllEvents?.Invoke(this, value);
    }

    public void Clear()
    {
      allHandlers.Clear();
      AllEvents = null;
    }
  }

Answer 2

no memory should leak when it is used correctly.

Adding and removing a handler results in an empty List<T> lingering in the Dictionary. It seems odd to use a list at all... (I'll come back to that).

---

dfhwze presents a good alternative solution which loads the complexity in one place. I would be inclined to consider the same, but perform conventional type checks instead of relying on dynamic. A simple change to dfhwze's code could be:

public void InvokeChange(T value)
{
    foreach (var listener in listeners)
    {
        if (listener is Action a)
            a();
        else if (listener is Action<T> aT)
            aT(value);
        else if (listener is Action<object> aObject)
            aObject((object)value);
        else
            throw new InvalidOperationException("<< Suitable exception text here, telling the user to shout at the maintainer >>")
    }
}

This requires no more repetition, and is more explicit about what is going on. It aso avoids dynamic, which makes it more maintainable because all dependencies are statically declared.

Using a sorted dictionary/list, you could put everything in the dictionary, and enumerate its values instead of maintain two collections (the dictionary and generic event). This would reduce redundancy (good), improve add/remove complexity, but increase the overhead of calling the event.

---

Personally I like the proxy approach, but I couldn't bare to have all that code repeated in the events: you might consider packaging it (and ObjectEventProxies) in a class somewhere so that you can reuse it tidily. That said, most of the code is spent implementing a dictionary which stores an ordered list of values (which is a perfectly generic data structure), so you could just throw one of those together, and that would reduce the amount of repetition (and so fragility) significantly without the effort and risk inherent in trying anything more interesting. This would leave the event handler as more like:

add
{
    Action<T> proxy = /* whatever */;
    WhateverEventProxies.Add(value, proxy);
    ChangeValue += proxy;
}
remove
{
    var proxity = WhateverEventProxies.Remove(value);
    ChangeValue -= proxy;
}

All of the complexity of managing the dictionary is gone, and now the intention of these accessors is clear.

One significant advantage of stuffing everything into another class would be that it would be easier to make thread safe; though, in this case, you could probably just use a thread-safe dictionary with a count, since any event handlers added more than once are equivalent (the ordering doesn't matter).

---

Since you are stuffing everything in ChangeValue to preserve the order, it's possible to add something with, for example ChangeObject only to remove it with ChangeValue, but since delegates provide exactly one method, there is no problem, and this means you can just use one dictionary (with object as the key) to track all the proxies in a single class.

Answer 3

Review

You wrap all event signatures into Action<T>. I would instead store all event listeners as their common base type Object. This way you don't need to create wrapper delegates.

add
    {
        // ..
        Action<T> proxy = (T v) => value.Invoke(v);
        eventProxies.Add(proxy);
        ChangeValue += proxy;
    }

The proxies seem overkill to me. No need for storing listeners in more than 1 backing collection.

private Dictionary<Action<object>, List<Action<T>>> ObjectEventProxies
    = new Dictionary<Action<object>, List<Action<T>>>();

Proposed Solution

There is an elegant solution that meets all of your contrived requirements which takes advantage of the runtime for finding method overloads using the dynamic keyword.

Store all listeners in a LinkedList (allow for duplicates).

 private LinkedList<object> listeners = new LinkedList<object>();

Register/Unregister event listeners without using boiler-plate code.

        private void Register(object listener)
        {
            if (listener == null) return;
            listeners.AddFirst(listener);
        }

        private void Unregister(object listener)
        {
            if (listener == null) return;
            listeners.Remove(listener);
        }

        public event Action<T> ChangeValue
        {
            add => Register(value);
            remove => Unregister(value);
        }

        public event Action<object> ChangeObject
        {
            add => Register(value);
            remove => Unregister(value);
        }

        public event Action ChangeEmpty
        {
            add => Register(value);
            remove => Unregister(value);
        }

Invoke event listeners using the dynamic keyword. The runtime knows which overload of InvokeChange to call. I have added one overload to take any Object as a fallback to avoid runtime exceptions, but this should be unreachable code. The combination of Reverse(), AddFirst() and Remove() ensures correct behavior. Performance is another thing though ..

        public void InvokeChange(T value)
        {
            foreach (var listener in listeners.Reverse())
            {
                Invoke((dynamic)listener, value);
            }
        }

        // these methods require equivalent method signatures ->
        // -> see comments that Action<T> and Action<object> can be combined!

        private void Invoke(Action<T> action, T value)
        {
            action.Invoke(value);
        }

        private void Invoke(Action<object> action, T value)
        {
            action.Invoke(value);
        }

        private void Invoke(Action action, T value)
        {
            action.Invoke();
        }

        private void Invoke(Object unsupportedEventListenerType, T value)
        {
            // ignore or throw exception ..
        }

Answer 4

Thank you for the answers that have been posted so far. Each contributed something that I wanted to incorporate in my second iteration of this problem.

Thank you specifically for solutions by

• dfhwze - using a single backing collection instead of one for each event
• Henrik Hansen - using reference counting to avoid needing a proxy List for each handler.
• VisualMelon - separating proxy management logic as its own object.

I've compiled my own approach that pulls from each of these answers

---

A Utility for Managing the event proxies

I'm realizing that this is a very specific use case - A single Action<T> event that should also fire as Action<object> and Action depending on what the listener needs - that I could see using in a few areas, and so chose to implement it as a separate utility class.

As was pointed out by Henrik Hansen, C#'s EventHandler is arguably preferable but its use would require changing the signature of the events and therefore the signature of any event handler functions that are Added to them, which in this specific case I am trying to avoid.

My implementation of the Event Proxy Utility object is:

public struct EventProxyContainer<T>
{
    private struct EventProxy
    {
        public Action<T> proxy;
        public int count;
    }
    private Dictionary<object, EventProxy> handlerProxies;

    public Action<T> Add(object handler) { /* See Below */ }

    public Action<T> Remove(object handler) { /* See Below */ }
}

Instead of performing the type check in the Invoke, I chose to handle that in the Add function itself. My intuition is that we will be invoking events more than adding them so this should give a bit of performance benefit.

This also means we can actually avoid having to construct a proxy at all for Action<T> handlers.

Add Implementation

public Action<T> Add(object handler)
{
    if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
    handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();

    EventProxy entry;
    if(handlerProxies.TryGetValue(handler, out entry))
    {
        entry.count += 1;
        handlerProxies[handler] = entry;
    }
    else
    {
        entry = new EventProxy() { count = 1 };
        if(handler is Action<object>)
            entry.proxy = (v) => ((Action<object>)handler).Invoke(v);
        else if (handler is Action)
            entry.proxy = (v) => ((Action)handler).Invoke();
        handlerProxies.Add(handler, entry);
    }
    return entry.proxy;
}

Remove implementation

Again we early out if the handler is Action<T>

public Action<T> Remove(object handler)
{
    if(!(handler is Action<object>) && !(handler is Action)) return (Action<T>)handler;
    handlerProxies = handlerProxies ?? new Dictionary<object, EventProxy>();

    EventProxy entry;
    if(handlerProxies.TryGetValue(handler, out entry))
    {
        entry.count -= 1;
        if(entry.count == 0) 
            handlerProxies.Remove(handler);
        else
            handlerProxies[handler] = entry;
    }
    return entry.proxy;
}

---

Foo<T> Implementation

This really cleans up the Foo<T> implementation quite nicely:

public class Foo<T> : IFoo<T>
{
    private EventProxyContainer<T> changeProxy;

    public event Action<T> ChangeValue = delegate {};

    public event Action<object> ChangeObject 
    { 
        add => ChangeValue += changeProxy.Add(value); 
        remove => ChangeValue -= changeProxy.Remove(value); 
    }

    public event Action ChangeEmpty 
    { 
        add  => ChangeValue += changeProxy.Add(value); 
        remove => ChangeValue -= changeProxy.Remove(value);
    }

    public void InvokeChange(T value) 
    {
        ChangeValue.Invoke(value);
    }
}

---

I like this approach because

• It still satisfies all the original requirements, and produces the same output as my original examples.
• It can be retrofitted in any case where you have an event of type Action<T> where you also want event listeners to be able to use it as Action<object> and Action instead.
• The proxy handling logic is well contained to a single utility object and separate from whatever else might exist in IFoo.
• Reference counting our proxies allows us to limit one memory allocation per Unique Handler
• We only construct a proxy for Action<object> and Action handlers - Action<T> handlers are added to the backing event object as normal.
• Add and Remove whitelist only Action<object> and Action, and return null in all other cases, which event += and event -= handles gracefully.

The updated code with examples can be found as a DotNetFiddle here, and as a gist here.