Addition of generic event arguments class

[Hawkeye4040]

Background and Motivation

I find myself writing basic event arguments classes often in order to wire up object changed events in my classes often. When no other data is needed I've found that a generic event arguments class works well here. Rather than storing them in a "core" library of mine it makes sense to add this particular class to the .NET Runtime itself.

Proposed API

namespace System
{
    public class GenericObjectChangedEventArgs<T> : EventArgs
    {
        public T OldValue { get; }

        public T NewValue { get; }

        public GenericObjectChangedEventArgs(T oldValue, T newValue)
        {
            OldValue = oldValue;
            NewValue = newValue;
        }
    }

    public delegate void OnGenericObjectChanged(object sender, GenericObjectChangedEventArgs e);
}

Usage Examples

    public class MyClass
    {
       private string _name;

       public string Name
       {
           get => _name;
           set
           {
               GenericObjectChangedEventArgs<string> e = new GenericObjectChangedEventArgs<string>(_name, value);

               _name = value;

              if (e.OldValue != e.NewValue)
                  NameChanged?.Invoke(this, e);
           }
       }

        public event OnGenericObjectChanged NameChanged;
    }

Alternative Designs

Prior to a generic event arguments class I would find myself often writing out classes needlessly to simply store an old value and new value. This one class can be used to fire events on property changes easily as shown above in the usage example. If other properties such as timestamps are needed either this class or EventArgs could be derived from.

On a side note a better class name could be used, I just haven't been able to think of one yet. I feel a better, more fitting name is out there for this class though.

Risks

No known risks.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[benaadams]

Could be a generic version of EventArgs?

namespace System
{
    public class EventArgs<T> : EventArgs
    {
        public T OldValue { get; }

        public T NewValue { get; }

        public EventArgs(T oldValue, T newValue)
        {
            OldValue = oldValue;
            NewValue = newValue;
        }
    }

    public delegate void EventHandler<T>(object sender, EventArgs<T> e);
}

[Hawkeye4040]

@benaadams Yeah I like that naming better I forget we could use EventArgs still if using a type parameter. I'll change that around in a few and commit that.

Why the type parameters on the delegate? Specifically the first one.

Pull Request #49436

[benaadams]

Why the type parameters on the delegate? Specifically the first one.

To specify the type of the value for consumers of the event

public class MyComponent
{
    public event EventHandler<string> NameChanged;
}

public class MyClass
{
    private MyComponent _component;

    public MyClass()
    {
        _component = new();
        _component.NameChanged += NameChanged;
    }

    private void NameChanged(object sender, EventArgs<string> e)
    {
        throw new NotImplementedException();
    }
}

[Hawkeye4040]

Oh right of course! Yeah I'm changing that right now and I'll submit that shortly.

[Hawkeye4040]

I messed up the earlier pull request so I closed it and remade it #49438.

[huoyaoyuan]

Personally, I don't think the EventArgs base class makes any sense. I'm also against the design of object sender, EventArgs e.
Being classes for event args may cost a lot.

[Hawkeye4040]

In some cases in may cost a lot but in those cases something else more suitable should be used. I do find this pattern very useful quite often though. I have seen many scenarios in which it shouldn't be used of course. But the same could be said for anything really. I don't always use just one pattern though. I use what is appropriate for that situation.

[Hawkeye4040]

Thank you @jeffschwMSFT I wasn't sure which area it fell under. So to clarify for the future anything under the system namespace would fall under that label?

[jeffschwMSFT]

@Hawkeye4040 area-System.Runtime is a little of a catch-all, but using the simple heuristic of things directly in the System namespace is a reasonable proxy.

cc @jeffhandley

[jeffhandley]

I wouldn't expect EventArgs<T> to have OldValue and NewValue; I'd probably expect it to just have a single Value property since nothing connotes "change" in the name. Looking for existing APIs that have NewValue, there are a lot of them though--each representing some type of change.

With OldValue and NewValue both having "Value" in the names, I propose System.ValueChangedEventArgs<T>.

[jeffhandley]

Also worth mentioning, but I don't love it myself. 😉

namespace System
{
    public class EventArgs<T> : EventArgs
    {
        public T Value { get; }

        public EventArgs(T value)
        {
            Value = value;
        }
    }
}

namespace App
{
    public class MyClass
    {
        private MyComponent _component;

        public MyClass()
        {
            _component = new();
            _component.NameChanged += NameChanged;
        }

        private void NameChanged(object sender EventArgs<(string OldValue, string NewValue)> e)
        {
            Console.WriteLine($"OldValue: {e.Value.OldValue}. NewValue: {e.Value.NewValue}.");
        }
    }
}

[benaadams]

Being classes for event args may cost a lot.

So struct so more like the following?

namespace System
{
    public struct ValueChangedEventArgs<T>
    {
        public T OldValue { get; }

        public T NewValue { get; }

        public EventArgs(T oldValue, T newValue)
        {
            OldValue = oldValue;
            NewValue = newValue;
        }
    }

    public delegate void ValueChangedEventHandler<T>(object? sender, ValueChangedEventArgs<T> e);
}

[Hawkeye4040]

Ha that's funny because ValueChangedEventArgs is what I had eventually and someone told me to change that. I sort of agreed but I think Value changed makes more sense here as well.

[Hawkeye4040]

I like the idea of a struct there. I haven't given that much thought yet but I will.

What about considerations surrounding value type vs reference type here?

[svick]

@benaadams Note that this goes against the Framework Design Guidelines:

✔️ DO use System.EventHandler<TEventArgs> instead of manually creating new delegates to be used as event handlers.

✔️ CONSIDER using a subclass of System.EventArgs as the event argument, unless you are absolutely sure the event will never need to carry any data to the event handling method, in which case you can use the EventArgs type directly.

If you ship an API using EventArgs directly, you will never be able to add any data to be carried with the event without breaking compatibility. If you use a subclass, even if initially completely empty, you will be able to add properties to the subclass when needed.

Not every API has to follow the FDG (especially for guidelines that are almost 20 years old), but I think violating them should have a pretty good justification.

[svick]

Also, this introduces a new API for notifications of property changes, when there already is one: INotifyPropertyChanged and its PropertyChangedEventArgs, with quite a different design. (INotifyPropertyChanged means one event per class, and has no information about values in the event args; the API proposed here means one event per property and has information about both the old and the new value).

Would it be better to extend INotifyPropertyChanged, instead of introducing a brand new API for this (this has already been proposed in #27252)?

[Hawkeye4040]

@svick I'm proposing something a little different here than extending INotifyPropertyChanged. This isn't a duplicate proposal, a similar one sure. By your own description this interface approach seems quite inferior.

As for the guidelines part. I hardly believe in following something that's 20 years old when over those years so much has changed. I'm not saying there aren't parts that are still relevant but it's also not going to be the programmers commandments either.

[Hawkeye4040]

I can't get the struct idea out of my head. I really like it for a lot of reasons. I want to say there's a reason not to do it that way but I'm drawing a blank on it. I've poured over many articles such as this one and read various manuals/publications covering the topic.

I really can't find anything wrong with it, I like it I don't see how it goes against the Framework Design Guidelines exactly. Can anyone clarify on that more? I feel like there's something off there I'm just forgetting.