`WithPropertyEvents` modifier to enable easy INotifyPropertyChanged implementations

[AnthonyDGreen]

Scenario

INotifyPropertyChanged is essential for modern UI patterns like MVVM. Today developers must add code to all of their properties to take advantage of the interface and that is repetitive and erodes the value of automatically implemented properties. Here is a typical pattern:

Property Name As String
    Get
        Return _Name
    End Get
    Set(value As String)
        If value = _Name Then Return

        _Name = value

        OnPropertyChanged()
    End Set
End Property

Protected Sub OnPropertyChanged(<CallerMemberName> Optional propertyName As String = "")
    RaiseEvent PropertyChanged(New PropertyChangedEventArgs(propertyName))
End Sub

Customers have been begging us for years for a solution to this problem but we've been reluctant for several reasons:

1. We're hesitant to bake a specific pattern into the language, as patterns can and do change.
2. We prefer general solutions to specific solutions and have been pursuing a general meta-programming feature that would encompass the INotifyPropertyChanged problem.

But, in the last VB LDM, VB MVP @KlausLoeffelmann was on campus as our guest and convinced us to get over it, so I'm pulling this proposal out of moth-balls.

Proposal

I proposal a new class modifier, tentatively WithEvents or WithPropertyEvents. When this modifier is present the code generated for auto-implemented properties will invoke a set of pseudo-partial methods. They're not true partial methods for several reasons, but are like partial methods in one particular way--calls to them are only emitted if they have been defined in user code. If user code implements these properties they'll be called. Users can easily leverage this to implement INotifyPropertyChanged without having to use expanded properties. It'll work something like this:

WithEvents Class Person

    Property Name As String     

End Class

Will translate to:

WithEvents Class Person

    <CompilerGenerated>
    <Hidden>
    Private _Name As String

    Property Name As String
        Get
            OnPropertyGet(NameOf(Name), _Name)
            Return _Name
        End Get
        Set(value As String)
            Dim oldValue = _Name
            If Not OnPropertySetting(NameOf(Name), oldValue, value) Then Return
            _Name = value
            OnPropertySet(NameOf(Name), oldValue, value)
        End Set
    End Property     

End Class

Essentially auto-props will now have well-known places where users can plug in other things. Normal overload resolution will be performed on the found methods so if the user wants to make things generic or strongly-typed they can.

The idea behind the OnPropertySetting call is two fold. It's very common for properties to disregard redundant sets to their current value. The implementer of OnPropertySetting can do several things:

• By returning False they could early exit the entire property.
• If the 3rd parameter (value) is ByRef they can change the value being set, e.g. (trim a string).
• Raise INotifyPropertyChanging.
• Log a change before it happens.
• Throw an exception if something is wrong.

In practice code leveraging this feature would look like this.

' Write once
Public Class ViewModel
    Implements INotifyPropertyChanged

    Public Event PropertyChanged As PropertyChangedEventHandler Implements INotifyPropertyChanged.PropertyChanged

    Protected Overridable Sub OnPropertyChanged(<CallerMemberName> Optional propertyName As String = "")
        RaiseEvent PropertyChanged(New PropertyChangedEventArgs(propertyName))
    End Sub

    Protected Overridable Function OnPropertySetting(Of T)(propertyName As String, oldValue As T, newValue As T) As Boolean
        Return Not Object.Equals(oldValue, newValue) 
    End Function

    Protected Overridable Sub OnPropertySet(Of T)(propertyName As String, oldValue As T, newValue As T)
        OnPropertyChanged(propertyName) 
    End Function

End Class

' In this new bright future most VB users would only need to write this class because the feature just looks up the method normally and all the plumbing can be implemented once in a base class.
WithEvents Class Person
    Inherits ViewModel

    Property FirstName As String
    Property MiddleName As String
    Property LastName As String
    Property DateOfBirth As Date

End Class

Undoubtedly there will be a case where someone wants to opt-out of this functionality. I propose that another new member modifier, NoEvents, can be specified on a property to opt it out of the default behavior.

Discussion points

• How do we avoid breaking changes? The feature requires opt-in through some syntax.
• Should it be a class modifier? Yes, specifying the modifier on every property would be annoying.
• If it's a class modifier, what should the keyword be? <Let's discuss>
• Does this work for structures? No reason to disallow.
• Does this work for Shared properties? No reason to disallow.
• Does this work for modules? No reason to disallow.
• Could we support other property "events" like:
  • OnPropertySetFault--called when an auto-prop throws an exception. Its existence causes the body of the auto-prop to be wrapped in a Try/Catch/Finally block.
• Could we support other members like auto-implemented events? No scenario for this.
• Could we generalize this further to support callbacks in any member body in a class, e.g. WithMethodEvents or OnMethodInvoked and OnMethodReturn to support logging or other instrumentation? <Let's discuss>
• Should add another layer of precedence to this feature by first looking for a specifically named member to call, e.g. OnFirstNamePropertySetting, OnFirstNamePropertySet, etc. and only calling the general method if a more specific method isn't implemented? This would allow for some special handling of one-offs, though arguable such functionality would best be written in the Get or Set method for that property.
• This doesn't solve the repetitiveness of declaring Dependency properties. I'm OK with that.

[franzalex]

I don't know about others but in the cases where I've had to implement INotifyPropertyChanged, there were just a few properties that it was needed on. For that reason, I'd rather prefer to have the keyword applied to the property not on the class in order to allow a more specific implementation as needed.

For example, I would rather opt in to properties to receive notifications on like this

Public Class Person
    Public Property WithEvents Name As String
    Public Property Age As Integer
    Public Property Gender As Gender
End Class

Instead of having to opt out of properties I don't want to receive notifications on.

That's my two cents on this issue.

[obelink]

I'm more a 'composition above inheritance' guy when possible. Is there a way we can avoid inheritance? I like the way Fody PropertyChanged is working with attributes/interface. Don't know how it's actually implemented though...

https://github.com/Fody/PropertyChanged

[AnthonyDGreen]

Inheritance is just an example. You could implement it on each class individually or use extension methods if you wanted. The feature really is 'compiler will call these methods at these times'. It's not particular about where the methods come from or what they do.

[esentio]

I'm with @franzalex and I would also prefer to have the keyword on property and not on the class.

Although I certainly welcome simplifying INotifyPropertyChanged properties, this is actually not what bothers me the most. I have custom snippets for these, so I write them quite fast with necessary if-changed-then-raise code in setter. But I use to have always quite a few readonly properties for which I want to raise PropertyChanged event as well.

One of my typical examples:

Public Class FooGridViewModel
  Implements INotifyPropertyChanged

  Private m_Records As List(Of Foo) = Nothing
  Public Property Records() As List(Of Foo)
    Get
      Return m_Records
    End Get
    Set(ByVal value As List(Of Foo))
      If Not Object.ReferenceEquals(m_Records, value) Then
        m_Records = value
        OnPropertyChanged(NameOf(Me.Records))
      End If
    End Set
  End Property

  Private m_SelectedRecord As Foo = Nothing
  Public Property SelectedRecord() As Foo
    Get
      Return m_SelectedRecord
    End Get
    Set(ByVal value As Foo)
      If Not Object.ReferenceEquals(m_SelectedRecord, value) Then
        m_SelectedRecord = value
        OnPropertyChanged(NameOf(Me.SelectedRecord))
        OnPropertyChanged(NameOf(Me.IsRecordSelected))
        OnPropertyChanged(NameOf(Me.CanEdit))
        OnPropertyChanged(NameOf(Me.CanDelete))
      End If
    End Set
  End Property

  Public ReadOnly Property IsRecordSelected As Boolean
    Get
      Return m_SelectedRecord IsNot Nothing
    End Get
  End Property

  Public ReadOnly Property CanEdit As Boolean
    Get
      Return Me.SelectedRecord IsNot Nothing AndAlso Me.SelectedRecord.Status = Status.Created
    End Get
  End Property

  Public ReadOnly Property CanDelete As Boolean
    Get
      Return Me.SelectedRecord IsNot Nothing AndAlso Not Me.SelectedRecord.Status = Status.Finished
    End Get
  End Property

  ' ...
End Class

So with this proposition, I would still need to write setter for SelectedRecord. And (worst part) I still need to track in which other property SelectedRecord is referenced and raise PropertyChanged event for it. With more complex code one can easily miss something. Using property from base class (inside readonly property) is "pain" as well.

I'm not sure if this is critical for others though. Or if the goal is also to solve such use cases. I also don't know if there is an easy universal solution for it. Fody seems to solve it, but to be honest I didn't try it yet, so I don't know how reliable it is with more complicated getters.

Maybe I was little bit off topic here, but as @AnthonyDGreen mentioned in #198, we have only one shot to make INotifyPropertyChanged less painful.

[AnthonyDGreen]

I mean, technically you could in your override of OnPropertyChanged Select on propertyName and raise the appropriate dependent property changed notifications. That seems like the most centralized place to do it. Actually having the language somehow understand dependencies between readonly and read-write properties seems... difficult and messy.

[reduckted]

I'd love to see this happen (though looking at #198, maybe that would be a better solution).

I like that specific properties can opt-out of this behaviour, because, when I need to raise PropertyChanged events, I occasionally have private properties in the same class and wouldn't want to have the event raised for those properties.

I'm also in agreement with @franzalex about having properties opt-in to this behaviour. Perhaps we could allow both styles? You could:

• Put the modifier on the class and opt-out on certain properties, or
• Not put the modifier on the class and opt-in on certain properties.

[bandleader]

+1 for having the WithEvents keyword on the property declaration itself and not on the class. If you do it on the class, you introduce the need for NoEvents, and you create ambiguity for partial classes. (And in any case it is less logical, since WithEvents has no effect on the class itself, only to turn each property to a WithEvents property, and that isn't so valuable just to save a single keyword on the property declarations.) However, if it is extremely common that people will create classes with every single property notifying, then WithEvents on the class does have merit.

Keyword

I would recommend against re-using the WithEvents keyword for this (i.e. WithEvents Property someProp As String). The behaviour isn't the same as WithEvents -- you won't be choosing the property from the drop-down now and writing an event handler that Handles someProp.SomeEvent. I think WithPropertyEvents is better, but creates weird repitition: WithPropertyEvents Property someProp As String. I propose as syntax Notifying Property someProp As String. This accurately describes the fact that the property will be notifying about changes -- implementing INotifyPropertyChanged.

Other options

This assumes that this proposal is indeed the best solution for this proposal. However, there are other suggestions, which have to be weighed carefully -- see #219. (The whole idea is basically a single lone sub-scenario of AOP...)

[reduckted]

@bandleader Check out #198. It's been rejected, but there's a lot of discussion in there about what keyword to use.

[AnthonyDGreen]

I've revised this idea a bit since the last time we discussed it in the VB LDM. Just haven't gotten the working prototype out there. I'm pretty excited about it.

[AnthonyDGreen]

Hey guys. I'm likely going to withdraw this proposal in favor for what's described by #282. Please check it out.

[jrmoreno1]

If possible I wouldn't put the keyword on either the class or the property, instead I would put it the same place that Handles does, on a method, so the new keyword should be HandlesPropertySetters , with a couple of pseudo events.

Class Person  
    Property Name As String     
    Property NickName As String  

    Protected Sub OnPropertyChanged_BeforeSet(propertyName As String, currentValue as String, ByRef pendingValue as String)  HandlesPropertySetters Name.BeforeSet
     'whatever you want to do before it is changed
    End Sub

    Protected Sub OnPropertyChanged_AfterSet(propertyName As String, oldValue as String, ByRef newValue as String)  HandlesPropertySetters Name.AfterSet, NickName.AfterSet
     'whatever you want to do after it is changed
    End Sub
End Class

Which the compiler re-rewrites as :

Class Person  
    Property Name As String
     Get
       Return _Name
     End Get
     Set
       OnPropertyChanged_BeforeSet(NameOf(Name), _Name, value)
      Dim oldValue = _Name
      _Name=value
      OnPropertyChanged_AfterSet(NameOf(Name), oldValue, _Name)
     End Set
    End Property

    Property NickName As String
     Get
       Return _NickName 
     End Get
     Set
      Dim oldValue = _Name
      _NickName =value
      OnPropertyChanged_AfterSet(NameOf(NickName), oldValue, _NickName )
     End Set
    End Property

    Protected Sub OnPropertyChanged_BeforeSet(propertyName As String, currentValue as String, ByRef pendingValue as String)
    'whatever you want to do before it is changed
    End Sub

    Protected Sub OnPropertyChanged_AfterSet(propertyName As String, oldValue as String, ByRef newValue as String) 
     'whatever you want to do after it is changed
    End Sub
End Class

Note that NickName doesn't have a before handler (and there should be a BeforeGet but this is long as it is), and this is also easily extensible if you want several BeforeSet or AfterSet handlers, just add them in lexicographical order (i.e. position in the source tree), and they get added in into the property in the right place.