Implement better properychanged event ?

[softlion]

Actual

        public event global::System.ComponentModel.PropertyChangedEventHandler? PropertyChanged;
        private void OnPropertyChanged(global::System.ComponentModel.PropertyChangedEventArgs eventArgs)
        {
            this.PropertyChanged?.Invoke(this, eventArgs);
        }

Better

        private void OnPropertyChanged([CallerMemberName] string? propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

Thoughts ?

[canton7]

Firstly, I'd say the first one is better. That way we can use a cache of PropertyChangedEventArgs instances to avoid allocating a new one each time.

Secondly, if you define your own void OnPropertyChanged([CallerMemberName] string? propertyName = null) method, PropertyChanged.SourceGenerator will use it.

[canton7]

The GC also puts long-lived objects into gen2, where they'll only very rarely considered for collection. This is a very normal caching strategy, and is the one used by MS in their PropertyChanged also (in the mvvm toolkit).

And again, as the readme says, if you want to use a PropertyChanged method with a different signature, just define it and this SG will use it.

[softlion]

Instead of sending me to hell, you could be less aggressive.

Mvvm toolkit is not a good source for good code. Ms does not so good code everywhere, we would have a perfect Windows if that was real 😂
Mvvm tooling is, in fact, the worst template I ever seen. You should not rely on it.

[softlion]

But I note that you are sending me to hell and I go back searching a new lib. It was a pleasure to discuss with you though.

[canton7]

I'm not sending you to hell. If you want to have a constructive conversation, then please don't talk down to me.

The second is more often used though without source generators.

Yes, but only because you don't have the advantage of a SG to generate the boilerplate for you.

One note on caching. Caching is not an easy service to implement correctly. It is often incorrectly implemented or missing basic features.

Caching without an expiry policy is easy. As long as you're not sticking huge objects into statics, caching objects for the lifetime of the application is very common and optimized for. Do you really worry about all of your static fields taking up memory?

So caching is beyond the goal of a propertychanged generator service, and should be opt-in and separated into another nuget.

Not viable. And as I've said, you can opt out of it very easily.

Another point is, the GC is optimized for cleaning small objects like args very efficiently.

I know. It's as optimal as they can be, but it's still relatively slow if you benchmark.

For example the new "record" keyword implements efficiently a way to prevent an object from being changed after being initialized once. Like the "init" setter of a get/set property.

I know what records are. Withers are efficient in terms of syntax, but still mean an object clone. I'm not sure how records are relevant here though?

This provides a safe way to post one object to multiple subscribers (ie: that's what an event source do) while being certain the object is not modified by any subscriber.

PropertyChangedEventArgs is immutable, so there's no harm in sending one instance to multiple subscribers.

My conclusion is, both versions could be generated safely, and it's up to the coder to choose the one that fit his way of doing things.

Indeed. As I said in response to your first comment, the coder can choose which one they want.

[canton7]

The cache is literally just:

public static class Cache
{
    private PropertyChangedEventArgs _foo;
    public PropertyChangedEventArgs Foo => _foo ??= new PropertyChangedEventArgs("foo");
}

This is fine, because PropertyChangedEventArgs is immutable. There's zero risk to keeping a single instance around and letting multiple people read it.

I think you're assuming that I have some complex system for reclaiming instances that aren't in use, then changing the PropertyName to something else and re-using them, but that's not the case.