fsprojects-archive / zzarchive-FSharp.Desktop.UI

F# MVC framework for WPF.
http://fsprojects.github.io/FSharp.Desktop.UI/
Other
81 stars 21 forks source link

[<DerivedProperty>] problem #6

Closed StefanBelo closed 9 years ago

StefanBelo commented 9 years ago

I noticed quite strange behavior for derived properties. In my model I have got:

[<DerivedProperty>]
member this.MarketName
    with get() = if this.OpenBetEvent.IsSome then this.Market.MarketInfo.ToString() else "No market selected"

[<DerivedProperty>]
member this.CanCloseMarket
    with get() = this.IsAuthorized && this.OpenBetEvent.IsSome

In the first initial data binding for my view CanCloseMarket is evaluated, but not MarketName. Market is abstract property. When OpenBetEvent changes correct values are set in bind controls on my view, but not on the first initial state.

My workaround is to not use DerivedProperty for MarketName and to call this.NotifyPropertyChanged "MarketName" whenever OpenBetEvent changes.

In my project I have got quite a lot of such models with this problem. In all such models one of properties is option value, so that could be maybe clue for you to solve this issue. I really do not know because as you can see in CanCloseMarket, the OpenBetEvent is option value and here the DerivedProperty works fine.

No errors or warnings are reported when bindings are set for those properties.

dmitry-a-morozov commented 9 years ago

As workaround use Nullable<_> type for optional values on models instead of option<_>. I can think of option<_> type support in data binding library. I'm curious: do you also bind OpenBetEvent directly to some control property or it only participates in data binding indirectly via derived properties?

StefanBelo commented 9 years ago

OpenBetEvent is not struct type so I cannot use Nullable in this case. As you suggested I tried to remove option type value from evaluation path in derived property, just from the if condition replacing this.OpenBetEvent.IsSome for IsValidMarket bool property, but it did not work.

I realized that in your evaluation process are included all used path.

this.OpenBetEvent, .IsSome this.Market .MarketInfo

and all of them must be valid when derived property is evaluated. Well, that is just my opinion you will shed more light on it.

So if I am right not even your Nullable solution would have been worked, because Nullable as well adds:

this.OpenBetEvent.HasValue, or .Value to evaluation path.

So what I had tried was to hide these dependency on not valid evaluation path in function. In another model I used:

[<DerivedProperty>]
    member this.MarketName
        with get() = GetMarketName(this.Market)

and Market is option value type. So MarketName is derived property dependent on Market option. This solved my problem.

Your suggestion to use Nullable values (for primitive values) whenever it is required by UI parts is valid. For instance, I noticed that double option value correctly worked in model binding to DataGrid column but not for negative values. What is quite strange, even negative value was displayed as positive number. What type of conversion was applied in this case is unclear for me.

dmitry-a-morozov commented 9 years ago

Properties of Nullable<_> type on a model has special treatment in my binding library https://github.com/fsprojects/FSharp.Desktop.UI/blob/master/src/DerivedProperties.fs#L13 https://github.com/fsprojects/FSharp.Desktop.UI/blob/master/src/DerivedProperties.fs#L20 https://github.com/fsprojects/FSharp.Desktop.UI/blob/master/src/Binding.fs#L110

This applied as you can see only to derived properties or single property expressions. .Value is always stripped out of a binding path. I can do the same for option<_> type. BTW, I still go back and force if .HasValue on nullable properties has to be removed from path too. Probably, yes.

StefanBelo commented 9 years ago

We want to program safely in F#, but we are not able to put away null at all because we need to interact with UI components that require CLI object.

You created Model type to create base model interface for UI implementing INotifyPropertyChanged and mainly you added AbstractProperties what allows us to use type values in properties where we need them to bind to UI control in null like manner.

Of course in F# we do not have null value for types, where we need it we must declare mutable option type in our models. As I said UI cannot live without null and so your IInterceptor returns null for not set properties, that is really nice. We can bind our F# type to WPF GridControl SelectedItem, actually option type would not work, and if your Model type was not here we had to use obj type for such properties in our models.

On the other hand we have safe state for our F# model properties, but not for those directly interacting with UI elements.

Your library have another nice feature, derived properties. Well as you add entire path to multibinding in my example

[<DerivedProperty>]
member this.MarketName
    with get() = if this.OpenBetEvent.IsSome then this.Market.MarketInfo.ToString() else "No market selected"

As there are safe parts and not so safe parts for binding, the initial state is not reflected correctly to WPF control, because this.Market is not set, so calling this.Market.MarketInfo.ToString() will fail.

For those who are not familiar with your library implementation the above code looks correct, as it assumes that if OpenBetEvent is None then "No market selected" must be returned, well of course it is returned correct value, but binded property MarketName is never called from UI update process as multibinging will fail in evaluating this.Market.MarketInfo.

So knowing how your library works I can create a code where I put for your multinbing only those properties which are actually dependent, derived property/ies, so just plain this.Market, and it represents safe data when evaluating in UI binding.

If my model generates INotifyPropertyChanged for Market property MarketName INotifyPropertyChanged will be executed as well, so UI will show correct state.

Maybe what you did, so adding all named path-es in expression is good idea, but on the other hand why not to minimize the number of multibinging nodes, and actually when you define your model with some properties, you declare that your model works on those properties directly, so just root properties should be included.

In my app domain Market.MarketInfo.Name represents MarketName but only Market property in the context of MVC model changes.

I hope you will understand what I want to say, English is not my mother language.

dmitry-a-morozov commented 9 years ago

I think that it's unique strength of derived properties that it supports hierarchical model. i.e. if you have nested model defined those dependencies still will be tracked. Seems like the best way to resolve the issue is to support option<_> in the same way as Nullable<_>. I will do it soon. As quick workaround instead of

[<DerivedProperty>]
member this.MarketName
    with get() = if this.OpenBetEvent.IsSome then this.Market.MarketInfo.ToString() else "No market selected"

Use

[<DerivedProperty>]
member this.MarketName
    with get() = if Option.isSome this.OpenBetEvent then this.Market.MarketInfo.ToString() else "No market selected"
dmitry-a-morozov commented 9 years ago

issue #6 fixed in 0.5.0-beta

dmitry-a-morozov commented 9 years ago

@StefanBelo Did it help?

StefanBelo commented 9 years ago

Unfortunately, your update did not fix this issue. In another model I have got:

let mutable myBetEvent : MyBetEvent option = None

and derived property:

    [<DerivedProperty>]
    member this.IsMyBetEventUpdateable
        with get() = 
            this.IsAuthorized && 
                match this.MyBetEvent with
                | Some value -> value.IsEditable
                | None -> false

Your Binding.OfExpression creates 4 multibinding nodes when model is bind to:

this.Root.bUpdateMyBetEvent.IsEnabled <- model.IsMyBetEventUpdateable

They are following: "" ".IsAuthorized" ".MyBetEvent" ".MyBetEvent.IsEditable"

This derived property works correctly when I hide what makes problem by doing so:

    [<DerivedProperty>]
    member this.IsMyBetEventUpdateable
        with get() = this.IsAuthorized && GetIsMyBetEventEditable(this.MyBetEvent)
dmitry-a-morozov commented 9 years ago

@StefanBelo Sorry for really delayed response - I was moving a whole family from NYC to California. As far as I understand then only wrong thing with multi-binding about is extra ".MyBetEvent.IsEditable", right? Question for you: would be helpful if Binding.OfExpression will return BindingExpression[] instead of unit. For debugging/diagnostics purpose?

StefanBelo commented 9 years ago

I am not so advance programmer in WPF XAML, so maybe what should help would be if you explain how the multibing works in your derived property implementation.

My naïve understanding is following:

this.Root.bUpdateMyBetEvent.IsEnabled <- model.IsMyBetEventUpdateable

So enabling or disabling toolbar button.

NotifyPropertyChanged "IsMyBetEventUpdateable"

is fired whenever any of multibinding path-es detects NotifyPropertyChanged.

If I am right, that in my case, the multibinding does not work for initial state because its evaluation is stopped when this path is evaluated:

".MyBetEvent.IsEditable"

MyBetEvent is option type, so it is set to null (in its initial state), and evaluation fails.

Your code actually works fine, but is not general for option types or Nullable values. In such cases it is better to solve it as I did, by creating some helper function to hide implementation details, and leave your derived property generation only for those properties, which in a context are safe for xaml multibinding and actually leaving in expression only those property names which should be used for multibinding.

If you think that you are able to find a general solution, then you can play with it, but I think it is not necessary. In your documentation just leave a comment that derived property works, as you said in hierarchical model, but only if all path-es included in multibinding are able to be safely evaluated in binding expression.

dmitry-a-morozov commented 9 years ago

Maybe a better way to re-phrase it: Nullable<_> or Option<_> value can be only the last in a path.

dmitry-a-morozov commented 9 years ago

@StefanBelo I deployed new version 0.6.0-beta. Try it out. The case you brought up is very tricky:

dmitry-a-morozov commented 9 years ago

@StefanBelo Sorry to bother you. I thought a lot about this issue. First, of course, derided properties is powerful but tricky feature and has some limitations. Second, as rule of thumb I still recommend that on model type all optional properties participating in data binding to be of type Nullable<_>. For simple reason that WPF data binding engine is aware of Nullable<_> and has no idea about option<_>. To keep F# code idiomatic and safe use conversion functions Option.OfNulllable and Option.toNullable (or extension methods if you prefer that) inside controller. You can use it inside derived properties too but probably it's not worth the hassle.

StefanBelo commented 9 years ago

Thanks Dmitry for all your help. This is my first F# project and I am just finding my way in functional programming.

Nullable in F# requires CLI or F# struct, so just from app domain design, I have got, it would require to change some types just to comply Nullable usage.

My backend was written strictly in F# way, but I had to change it, as you suggest, all generic option values to Nullable ones because of strange behavior for generic option values when binding to DataGrid columns.

Well if F# team come with Option type it would be nice if on CLI level this type behaves like Nullable, mainly for binding purposes in WPF.

When I started project despite of not having design support for WPF projects in F# I included xaml files to my project just by copying them from C# project, adding all references as well, but I was not able to make it run, so I ended like you did in your fsharp wpf mvc series, WPF is in C# project.

I do not know what others experiences are or your own ones when working with F# on WPF projects, mainly when using third party WPF libraries. I was not able to find other WPF projects in F#, just to check how others design such projects.