dotnet / vblang

The home for design of the Visual Basic .NET programming language and runtime library.
289 stars 65 forks source link

Bindable Classes and Properties #198

Closed AnthonyDGreen closed 6 years ago

AnthonyDGreen commented 6 years ago

Klaus said #194 was complicated by its generality. On the far end here is an alternative.

We only get one shot at this so whatever code pattern we pick better be all anyone ever needs.

===11/6/2017=== I've been thinking about another common scenario, computed properties which depend on other properties and need to fire PropertyChanged whenever their dependencies change. There are a few ways we could deal with this.

The imperative way Under the above proposal here's what a user could do.

Bindable Class Rectangle
    Implements INotifyPropertyChanged

    Event PropertyChanged As PropertyChangedEventHandler Implements INotifyPropertyChanged.PropertyChanged

    Property Width As Integer
    Property Height As Integer

    ReadOnly Property Area As Integer
        Get
            Return Width * Height
        End Get
    End Property

    Sub OnPropertyChanged(propertyName As String)
        RaiseEvent PropertyChanged(Me, New PropertyChangedEventArgs(propertyName))

        Select Case propertyName
            Case NameOf(Width), NameOf(Height)
                OnPropertyChanged(NameOf(Area))
        End Select
    End Sub

The declarative way That's doable but not ideal. But what if we made a way to declare such a dependency explicitly in code?

    ReadOnly Property Area As Integer DependsOn Width, Height
        Get
            Return Width * Height
        End Get
    End Property

Similar to the Handles clause for methods, the DependsOn clause could be added to the end of a expanded ReadOnly property (this is the only case for which this feature would make sense) and would in essence cause a special handler to be registered on the PropertyChanged event on object creation that would re-raise the event for the dependent property.

Bonus If the property being depended upon itself implements INotifyPropertyChanged or INotifyCollectionChanged a handler will also be registered for those events to raise PropertyChanged for the dependent properties. Of course, I think for this to make sense the compiler code-gen for the auto-props would have to be very similar to what we do for Handles today, where the handler is registered when the property is set and removed when the value changes. I think it might be common for composite ViewModels which aggregate other ViewModels or multiple model objects.

Likely bridge too far We could allow a single dotted name so that the user could be more granular about which sub-properties they depend on, e.g. DependsOn Product.UnitPrice. The only benefit of this would be performance and I don't think the frequency of that concern justifies the complexity, but let's discuss.

obelink commented 6 years ago

I like this approach!

KlausLoeffelmann commented 6 years ago

I like this better than #194. Said that, Cory raised an important issue: What, if you wanted to exclude one or a series of property from the auto-generation process. This is VERY important in real world scenarios. And it was one big argument that we implemented our prototype based on attributes and not with a new keyword (modifier). I would like a keyword better, if this would not be an issue, BTW.

Here is our current implementation with Attributes. The prototype allows to exclude specific properties with

<UserInterface(use:=false)>

autonotifyproperties

obelink commented 6 years ago

As I understand it correctly you can opt out a property by using the modifier NotBindable. Like this:

Public Bindable Class Person

    Public Property FirstName As String
    Public Property LastName As String
    Public NotBindable Property BirthDate As DateTime

End Class
AnthonyDGreen commented 6 years ago

@obelink speaks true. In the spirit of NotOverridable and NotInheritable we can have a modifier to opt out. So you have both a per property opt-in model and a per property opt-out model depending on your needs.

reduckted commented 6 years ago

I certainly like this approach better than #194. Having said that, I think that overall this is very similar, so we wouldn't be losing much by going with this approach over #194.

To make it a little less specific to INotifyPropertyChanged, perhaps we could allow this approach to work with INotifyPropertyChanging interface as well. So this:

Bindable Class Person
    Implements INotifyPropertyChanging
    Implements INotifyPropertyChanged

    Property Name As String
End Class

would translate to:

Class Person
    Implements INotifyPropertyChanging
    Implements INotifyPropertyChanged

    Property Name As String
        Get
            Return _Name
        End Get
        Set
            If Not Object.Equals(_Name, Value) Then
                OnPropertyChanging("Name") ' <-- only emitted if the class implements INotifyPropertyChanging.
                _Name = value
                OnPropertyChanged("Name")  ' <-- only emitted if the class implements INotifyPropertyChanged.
            End If
        End Set
    End Property     

End Class

To be honest though, I've never needed to use INotifyPropertyChanging, and the only place I've seen it used was in LINQ-To-SQL (which is now dead, and even then I think it was optional 🤔).

You mention in #194:

This doesn't solve the repetitiveness of declaring Dependency properties. I'm OK with that.

I think I'm OK with that too, and while you can't do anything about declaring the actual DependencyProperty field, perhaps the encompassing property could become auto-generated using this same technique.

For example, when the Bindable keyword is used on a class deriving from DependencyObject, this:

Public Shared ReadOnly NameProperty As DependencyProperty = DependencyProperty.Register(...)

Public Bindable Property Name As String

could be translated to:

Public Shared ReadOnly NameProperty As DependencyProperty = DependencyProperty.Register(...)

Public Property Name As String
    Get
        Return DirectCast(GetValue(NameProperty), String)
    End Get
    Set
        SetValue(NameProperty, Value)
    End Set
End Propery

There's more intricacies to dependency properties though (what about read-only dependency properties? And attached properties don't use properties, so you'd still need to manually define the GetName and SetName methods 😢), so maybe trying to cover dependency properties with this feature is biting off more than we should.

AnthonyDGreen commented 6 years ago

@reduckted I added a note to talk about INotifyPropertyChanging. @KlausLoeffelmann I added another layer to the proposal for declaratively marking dependencies between computed properties. Please take a look.

AdamSpeight2008 commented 6 years ago

This currently can be implement via an extension method / helper method. Gist

AnthonyDGreen commented 6 years ago

But it can't. The point is that the compiler generates the code automatically. What you're referring to is the generated code. The user code is just a regular auto property.

AdamSpeight2008 commented 6 years ago

@AnthonyDGreen I know that, it was more about how little code (once the extension method is defined) to implement INotifyChanging and INotifyChanged If VB has a similar concept to C#'s Expression Bodied Functions and Properties

Public Property Name() As String => Sub(Value) Me.SetValue(NameOf(Me.Name), Value, _Value )
AnthonyDGreen commented 6 years ago

That's pretty horrible.

KlausLoeffelmann commented 6 years ago

I agree with @AnthonyDGreen , @AdamSpeight2008: that IS pretty horrible.

My problem is the term Bindable/NonBindable still. Especially for VB. I'm afraid a modifier named Bindable - although it has a similar mechanism underneath based on INotifyPropertyChanged - implies especially to a VB guys that exclusively the old WinForms Databinding is meant, instead of using it in MVVM scenarios with UWP, WPF, Xamarin, etc. UserInterface is just a broader term, which feels more to be in the original spirit of VB describing the keyword's purpose like Widening and Narrowing for Casting Operator Overloading instead of Explicit or Implicit in CSharp. Or the Extension Attribute instead of using this for Extension methods (although Extension should have always been a keyword and never an attribute!)

I think UserInterface may be encouraging people to use it in other technologies than WinForms, make curious what it is, and lead them to explore other things than they already know for 15 years. More importantly: I would like to suggest to establish a set of UserInterface supporting code synthesis I have in mind, and coming up for a new keyword for each of them ... hmm ... wouldn’t that kind of keyword-spam the language? Independent in our current prototype is consistently used to exclude code synthesis from something. UserInterface is used to “do” something User Interface related: synthesizing Property Changed Infrastructure when used on Properties, marshaling to the UI thread when used in Social (Async) scenarios. We have a couple of more ideas for that!!

And what's also important, or maybe my biggest issue: Conventional Auto-Implementing Properties are bindable in Mvvm Scenarios. They just do not update the UI when their content change, but their value IS taken into account on assigning the ViewModel. Bindable implies for me, that a conventional property is NOT bindable, which is not at all true. UserInterface, however, tells me that the property is optimized for the usage in such scenarios.

Here, btw, is a GIF that shows from the current state of our prototype how it would feel to code with it:

autouserinterfaceproperty_onclass

KlausLoeffelmann commented 6 years ago

This, BTW, is the result of the reflectored Assembly with Telerik's JustDecompile:

image

And, of course...

image

obelink commented 6 years ago

Great work!

To be honest, for me the naming thing is not a very big deal, however I prefer Bindable/NotBindable. I don't see the point that 'especially to VB guys' that there is an association with WinForms only. When I look at Xaml for example Xamarin, WPF or UWP the term Binding is still extensively used. There is someting like BindingContext and to the keyword Binding is also used to bind properties or commands. You bind a Bindable property to a Label etc. For me it fits perfectly.

Just my 2 cts.

But again... I'm very impressed what you have done so far. My compliments!

KlausLoeffelmann commented 6 years ago

Which name would you prefer for Social methods (and yes, there will be discussions about that name, either! :-) to indicate that you want ConfigureAwait(True)? We picked also UserInterface to indicate, this method is manipulating either a ViewModel property or a View (Form) directly. Would you leave that or introduce a 3rd keyword?

We have now:

We would end up with

I, personally, find those too many. Probably reverting back to Attributes? After all, there is also no Extension Keyword for Extension methods, but an Extension Attribute?

reduckted commented 6 years ago

It's nice that the only thing left to work out is the naming! 😆

implies especially to a VB guys that exclusively the old WinForms Databinding is meant

I don't see why calling something "bindable" would imply to a VB developer that it's for WinForms. I haven't touched WinForms in years, and I'm sure I'm not alone.

like Widening and Narrowing for Casting Operator

Off topic, but I hate these names. I can never remember which one I'm supposed to implement to get the behavior I want! 😕

I think UserInterface may be encouraging people to use it in other technologies than WinForms, make curious what it is, and lead them to explore other things than they already know for 15 years.

Honestly, I don't think the keyword UserInterface will help developers move away from WinForms and explore other technology - after all, WinForms is a user-interface technology!

I also dislike the pairing of UserInterface and Independent. It's not intuitive and doesn't fit with the X/NotX approach of other keywords (Overridable/NotOverridable for example).

And INotifyPropertyChanged is not exclusive to user interfaces (though that is where it's predominantly used).

I'm happy with Bindable, but if we're looking for alternatives, why not describe what the property does - the property notifies about changes:

Public Notifying Property Foo As Integer

¯\_(ツ)_/¯

Anyway, like I said before, if the naming is the only thing left to worry about, then that's a great problem to have! 😄

KlausLoeffelmann commented 6 years ago

I like Notifying better than Bindable! How would you exclude Properties in Notifying Classes from being Bindable? NotNotifying? NonNotifying? Silent?

So, like this?

Public Notifying Class ViewModel
    Public Property MvvmProperty As String
    Public Property AnotherMvvmProperty as Integer
    Public NonNotifying Property ConventionalProperty as DateTime
End Class
reduckted commented 6 years ago

NotNotifying would probably be the best fit (even though it is a bit of a mouthful!)

AnthonyDGreen commented 6 years ago

I see the point about all properties being bindable and that these really just have two-way binding. But as @obelink says there's this very strong connection to XAML "{Binding }". Another idea to throw on the heap. By analogy with ObservableCollection(Of T), which is what one should use when making a collection which should raise INotifyCollectionChanged to work with ItemsControl types we could use the terms Observable and NotObservable. That way you're talking in terms of observable classes and observable collections, not user-interface classes and observable collections.

It also connects well to the Observer Pattern which precisely describes what this is about:

The observer pattern is a software design pattern in which an object, called the subject, maintains a list of its dependents, called observers, and notifies them automatically of any state changes...

I actually think the term is a little... obscure but the whole point of design patterns is to give us shared vocabulary for common patterns (like MVVM).

Notifying and NotNotifying are growing on me a little (will have to noodle on them). It is a mouthful and still has the disconnect between notifying classes and observable collections. BTW Silent as the opt-out modifier is temptingly cool.

We did talk about several of these proposals in the VB LDM last week. I'll put up the notes on the discussion shortly. We didn't pick keywords, but we did compare approaches between this (which is essentially Klaus's, I think), #194, and #107 as solutions to the problem.

Bill-McC commented 6 years ago

Like Notifying, NonNotifying.

Dislike bindable and userinterface.

Should there be a Changing that allows cancelling ? Interruptible/Uninterruptible?

AnthonyDGreen commented 6 years ago

Cancellation is not part of the INotifyPropertyChanging interface actually. I think it just let's listeners grab a snapshot of current values before they change.

Bill-McC commented 6 years ago

Yep. Could throw an error: that'd interrupt it; but not nice. Is there no standard to implement for validation external to the class?


From: Anthony D. Green notifications@github.com Sent: Thursday, November 23, 2017 3:51:39 PM To: dotnet/vblang Cc: Bill-McC; Comment Subject: Re: [dotnet/vblang] Bindable Classes and Properties (#198)

Cancellation is not part of the INotifyPropertyChanging interface actually. I think it just let's listeners grab a snapshot of current values before they change.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/vblang/issues/198#issuecomment-346531748, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AID_hNfZpck269nLNzxwJdTjgrdsjpWEks5s5PnbgaJpZM4QPz9B.

reduckted commented 6 years ago

Is there no standard to implement for validation external to the class?

If you're in WPF land, it's INotifyDataErrorInfo

reduckted commented 6 years ago

It also connects well to the Observer Pattern which precisely describes what this is about

Observable Property isn't too bad, but there's already IObservable(Of T) and the whole Reactive Extensions that go along with it. It could be a bit misleading.

BTW Silent as the opt-out modifier is temptingly cool.

I agree. The only thing that bothers me about it is the "asymmetry" with the opt-in keyword. To be fair, though, I think the only other keyword pairing that's symmetrical is Overridable/NotOverridable. All of the other "opt-in/out" keywords don't have inverses - NotInheritable, MustInherit, Overrides. I'm warming to Silent. 😄

AnthonyDGreen commented 6 years ago

I was half-joking. If it were Observable it would be NotObservable. It's unfortunate that IObservable(Of T) is a push-based collection/stream and ObservableCollection(Of T) has nothing to do with it at all. Since an ObservableCollection "Represents a dynamic data collection that provides notifications when items get added, removed, or when the whole list is refreshed." by analogy an Observable property is one "that provides notifications when its value is changed."

But yeah, I do like that we're basically down to the naming things part of the feature. I feel good that we're going to tackle this.

esentio commented 6 years ago

My two cents:

I certainly welcome DependsOn for computed properties. And I would like to see single dotted names allowed. Even more complex paths like DependsOn Product.ProductDescription.Label would be great in some rare cases, but I understand nobody will want to go down this rabbit hole.

obelink commented 6 years ago

After a couple of days of sinking in these terms... Notifying/NonNotifying is the pair I like the most...

AdamSpeight2008 commented 6 years ago

Another possible pair to throw into the mix Eventing / NonEventing

DualBrain commented 6 years ago

Just to throw this in:

I tend to think that Bindable/NonBindable are (so far) the best choices; please allow me a moment to explain.

The title of this thread is Bindable Classes and Properties...

If one were to rename the title to Notifiable Classes and Properties; seems to me to sound a little odd.

In other words, whatever the keywords end up being, we do still have to discuss this in the general public and the relationship between what it's called and the keywords utilized needs to be considered.

Bindable Class, Bindable Properties Binding Class, Binding Properties Notifiable Class, Notifiable Properties Notifying Class, Notifying Properties

Additionally, as pointed out, these aren't always user interface related; there are many cases where binding, notifying, eventing could be non-UI.

So back to the original point, binding is the act of SETTING UP the POSSIBILITY of notification; something being bindable doesn't, in and of itself, mean that is "notifying" anything (or requires notification). It states that it "can".

In other words, can creating a class with these "connections" require being "notified" to participate, or could the class be utilized without any notification even though it's capable of being used with notifications? Or are we trying to create something here that REQUIRES notification?

I'm just trying to be sure that the naming of these isn't so focused on the goal (UI) that we forget generalization is one of the key points of language design.

With that said, I'm keen to stick with the Bindable / NonBindable keywords as these, to me, better describe the overall intent of that that the class/properties can be bound to for whatever purposes be that notifications or "whatever might come next" (something that is hard, if not impossible, to determine at the time of the original design).

obelink commented 6 years ago

This makes sense... Notifying/NotNotifying describes at best what the class and their properties are doing after applying these keywords. Bindable/NotBindable describes at best the purpose of these keywords. It matches perfectly the other related keywords at the 'consumer' site of the 'Bindable' classes.

It's quite hard! ;-)

Bill-McC commented 6 years ago

Isn't any public property bindable? We're not changing getters, only the setters to notify anything that does bind to refresh their binding.

KlausLoeffelmann commented 6 years ago

Exactly. Also, what other scenarios you have in mind for the TYPICAL not-CSharp but VB Developer, where she uses INotifyPropertyChanged for classes other than for UserInterfaces?

Bill-McC commented 6 years ago

You mean like when you open the garage door it notifies the house to turn on thw things you usually like? IOW never limit yourself to yesterday. UserInterface to me mean IDataError or whatever it is, validation, interaction.

AnthonyDGreen commented 6 years ago

Update: The 12/6/2017 LDM rejected this idea in favor of #194, which is more general.

AdamSpeight2008 commented 6 years ago

@AnthonyDGreen Would you mind adding tags for [LDM Rejected] on the these recent topics.