C# One type to represent multiple types (including generics)

Question

Our monolithic WinForm application is getting a face lift. One current challenge that we are restructuring how we setup events. I rolled my own event manager class to handle the subscriptions and unsubscriptions to prevent leaks whenever developers write code.

I ran into several challenges with storing generic values in non-generic types. So I came up with a pattern that allowed me to do it. Not sure if I am creating a code smell while doing it so I wanted someone to review and ask questions.

Base View

public abstract class View : Form, IView
{
    protected readonly EventManager EventManager;

    public View()
    {
        EventManager = new EventManager(this);
    }

    protected abstract void RegisterEvents();
}

Concrete Class

public class MyView : View
{
    protected override void RegisterEvents()
    {
        EventManager.Attach(button1_Click, handler => button1.Click += handler, button1.Click -= handler); 
    }
}

Event Viewer

public class EventManager : IDisposable
{
    private IList<TrackedEvent> Events = new List<TrackedEvent>();
    private IDisposable Owner;

    public EventManager(IDisposable owner)
    {
        Owner = owner;
    }

    public void Attach<TEventArgs>(EventHandler<TEventArgs> handler, Action<EventHandler<TEventArgs>> addEvent, Action<EventHandler<TEventArgs>> removeEvent = null)
    {
        Events.Add(GenericTrackedEvent<TEventArgs>.Register(@delegate, handler.Method.Name, removeEvent));            
    }
            
    public void Attach(EventHandler handler, Action<EventHandler> addEvent, Action<EventHandler> removeEvent = null)
    {           
        Events.Add(TrackedEvent.Register(@delegate, handler.Method.Name, removeEvent));           
    }

    public void Dispose() { ... }

    private class TrackedEvent : IDisposable
    {
        protected EventHandler Handler { get; set; }
        protected string MethodName { get; set; }
        protected Action<EventHandler> Unsubscription { get; set; }

        protected TrackedEvent() { }

        private TrackedEvent(EventHandler handler, string methodName, Action<EventHandler> removeEvent)
        {
            Handler = handler;
            MethodName = methodName;
            Unsubscription = removeEvent;
        }

        internal virtual void Remove()
        {
            Unsubscription(Handler);
        }

        internal static TrackedEvent Register(EventHandler handler, string methodName, Action<EventHandler> removeEvent)
        {
            return new TrackedEvent(handler, methodName, removeEvent);
        }

        public void Dispose()
        {
            Dispose(true);
        }

        protected virtual void Dispose(bool disposing)
        {
            Handler = null;
            Unsubscription = null;
        }
    }

    private class GenericTrackedEvent<T> : TrackedEvent
    {
        protected new EventHandler<T> Handler { get; private set; }
        protected new Action<EventHandler<T>> Unsubscription { get; private set; }

        internal override void Remove()
        {
            Unsubscription(Handler);
        }

        private GenericTrackedEvent(EventHandler<T> handler, string methodName, Action<EventHandler<T>> removeEvent)
        {
            Handler = handler;
            MethodName = methodName;
            Unsubscription = removeEvent;
        }

        internal static TrackedEvent Register(EventHandler<T> handler, string methodName, Action<EventHandler<T>> removeEvent)
        {
            return new GenericTrackedEvent<T>(handler, methodName, removeEvent);
        }

        protected override void Dispose(bool disposing)
        {
            Handler = null;
            Unsubscription = null;

            base.Dispose(disposing);
        }
    }
}

So several things to point out and as to why:

1. Some methods have been shortened for brevity.
2. TrackedEvent holds onto the unsubscription e.g: the removeEvent handler.
3. Previously implementations I struggled with storing generic types in a non-generic class without constant boxing/unboxing.
4. The two static methods "Register" will return a TrackedEvent class because its constructors are private. I don't want anyone creating one outside of my implementation.
5. TrackedEvent only handles non-generics.
6. GenericTrackedEvent handles generics by hiding our base properties and overriding key properties.
7. Unexpectedly, inheritance is working correctly when calling the remove method. My initial expectation was the GenericTrackedEvent was going to use the underlying TrackedEvent.Unsubscription and TrackedEvent.Handler properties but I was wrong - and that was okay.
8. Someone is going to ask - why are you passing over the MethodName? Because I actually wrap my handlers in a new delegate so I can capture when event fire and log them to any source I choose. When I wrap my delegates in the attach method it carries that name over instead of the original one passed in the handler parameter. Instead of masking that method, I figured I'd keep it. Again some code was remove because I do not deem it relevant to the initial question.

Seems like we've implemented a duck typing but its usage is abstracted away from developers so I think its okay but wanted several other opinions to weight in.

Thoughts?

Answer 1

I would question what you intend with this system. In my personal experience, when someone writes something like this it's because they have no actual work assigned to them, but don't want their managers to realize that.

Take a step back and ask yourself what is the improvement you gain from:

1. Requiring an additional class to be present in every form inheritance tree.
2. Making it so user controls aren't handled by this system (from the inside I mean, since your base class derives from Form).
3. Requiring a manual line of code to handle creation and removal of events in one specific function (what if I want to add a handler at run-time? What if I want to use post-2002 lambda expressions instead of 6 lines of code for a simple handler? What if I want to use the designer, which handles renaming for me?)
4. I don't think you even considered early event removal.

You say you want to avoid forgetting to remove event handlers. Any properly written handler doesn't need to be removed, only handlers that strongly root the form object require this (and in this case it's not a memory leak exactly, it's your form not closing). And this system doesn't solve strong rooting, since it stores a non-weak-referenced list of these unsubscribe handlers, which reference your form, thus rooting the form in your object. You're literally creating the problem you're claiming to solve (refer back to my first statement).

You don't show this, but what triggers the unsubscribe? The form's own dispose call? I hope not, because that won't trigger in a problem scenario with this code, and the fact that you mark this "manager" as IDisposable worries me greatly. The FormClosing event? I hope not, because that would prevent close cancellation. Yet another manual call to something? I hope not, because you're supposedly trying to automate ..something, not throw in more manual calls that always have to happen or BOOM.

As to the actual code presented, it's not much, but at the very least write a bit of smart code to handle subscribing and unsubscribing automatically from a handle to the event and handler. You have expressions to parse code provided to your function and generate changed executable code from it, and you have code emitting, between those two you should be able to call your function like this at least:

EventManager.Attach(button1_Click, button1.Click);

Edit: Oh god I just noticed your new field modifier. Use proper OOP and you won't need a GenericTrackedEvent<T> that's literally twice as large as the thing it "derives" from, since it has the previous fields as well as the new ones. And at the very least don't mark them protected, there's no reason for that.

Answer 2

Look into the C# dynamic type which indicates that the variables type can change at runtime.

For example,

dynamic event= "get_credentials";
...
event= 5;
...
event= someFunction;