Closed JDiLenarda closed 5 years ago
Can you provide an example of when this might be useful? I'm having a hard time thinking of a use-case where you would use a single subModelSeq
binding to set up significantly different items.
Let’s just take my use case. I’m writing an inspection application for civil engineering purpose. Users have to state defects on buildings, like cracklings or spots of corrosion. Those defects have different characteristics of different kinds : crackling has a lenght (formated decimal value in a textbox), a wetness state (choice in a combobox) and a comment (string in textbox) ; spot of corrosion has a number (integer value in a textbox), a rusty state (boolean in a checkbox), an area (decimal in textbox) and a comment.
User picks the defect type in a combo and its characteristics are displayed. Those characteritics with different models (or rather union cases) and different xaml controls are bound through a Binding.subModelSeq
.
Thanks! If I understand correctly, there are different defects, which are modelled by different F# types or different DU cases, but you want all of them in the same ListView, possibly using different item templates based on the type. Is this correct? (Edit: Different templates based on the type probably wouldn't work, because all view models are the same type, so nevermind that. But please let me know if the other stuff is correct.)
Your request is not unreasonable. But first, let's explore other options and see if there are other ways of solving this. We definitely want to avoid the XY problem here, particularly when we're considering making the API more complex.
My first thought would be to model the defects as a discriminated union, which I guess you are already doing. Then, in the binding list, you necessarily have to match on the model in some way or another. I would first try to just match on the model in each binding in the list, and just return null
if the binding is for a non-relevant union case. Could that work? I can't see the non-relevant bindings being present in the list causing any kind of performance issue, but I could be overlooking something.
Thanks! If I understand correctly, there are different defects, which are modelled by different F# types or different DU cases, but you want all of them in the same ListView, possibly using different item templates based on the type. Is this correct? (Edit: Different templates based on the type probably wouldn't work, because all view models are the same type, so nevermind that. But please let me know if the other stuff is correct.)
In fact these are the characteristics I want to put in an item list. I can’t hard code defects, because they are user defined. I can however hard code characteristics types. But anyway, replace defect with characteristic and you nailed it.
Your request is not unreasonable. But first, let's explore other options and see if there are other ways of solving this. We definitely want to avoid the XY problem here, particularly when we're considering making the API more complex.
My first thought would be to model the defects as a discriminated union, which I guess you are already doing. Then, in the binding list, you necessarily have to match on the model in some way or another. I would first try to just match on the model in each binding in the list, and just return
null
if the binding is for a non-relevant union case. Could that work? I can't see the non-relevant bindings being present in the list causing any kind of performance issue, but I could be overlooking something.
To be clear I already have a working prototype with current Elmis.Wpf version, so my Y is solved without X.
Model matching are done in the getters from the Binding.*
functions, which make them unfit for lambda expressions. On an unexpected match I throw a failwith
because my various UserControl
s bind different properties with different names, so I know a checkbox will never ask for an item list.
That works fine but it just occured to me that having 5 or 6 fitting binding lists with 2-4 items each would be neater than having only one of 20 items. Going forward, it would basically allow me to write a different, focused MVU module for each characteristic type (they’d have to share the same messages, though), with an upper MVU using a DU to pick the good one depending on the model. And bindings with short getters again.
It may improve performance, but it’s not a concern for my 6-10 characteristics in a pop-up.
Thanks for the clarification! I understand better now, but still not fully. Would you be able to share with me the code for your bindings list? It doesn't have to be exactly what you have, but something that is sufficient to get across the point regarding how the different bindings match different types. I think that seeing code for your concrete use case would help my understanding and allow me to better implement the correct solution.
Here's something simplified :
[<RequireQualifiedAccess>]
module Toggle =
type Model = { Name:string ; Value: bool }
let updade value model = { model with Value = unbox value }
[<RequireQualifiedAccess>]
module ItemChoice =
type Model = { Name: string ; Value: string option ; Items: string list }
let updade value model = { model with Value = unbox value }
[<RequireQualifiedAccess>]
module Numeric =
type Model = { Name: string ; Value: decimal option }
let updade value model = { model with Value = unbox value }
module Defect =
type Kind =
| Toggle of Toggle.Model
| ItemChoice of ItemChoice.Model
| Numeric of Numeric.Model
let nameOf = function
| Toggle mdl -> mdl.Name
| ItemChoice mdl -> mdl.Name
| Numeric mdl -> mdl.Name
type Model = { Name: string ; Characteristics: Kind list }
type SubMessage = | ChangeValue of obj
type Message =
| ChangeCharacValue of string * SubMessage
let updateKind charac name value =
if (nameOf charac) <> name then charac
else
match charac with
| Toggle mdl -> Toggle (Toggle.updade value mdl)
| ItemChoice mdl -> ItemChoice (ItemChoice.updade value mdl)
| Numeric mdl -> Numeric (Numeric.updade value mdl)
let update msg model =
match msg with
| ChangeCharacValue (name, value) ->
{ model with Characteristics = model.Characteristics |> List.map (fun charac -> updateKind charac name value) }
let characBindings () =
[
// binds to the DataTrigger property picking the UserControl to use
"CharacType" |> Binding.oneWay (fun mdl -> (* code retrieving an enum value*) )
// binds to the characteristic name, shared by all UserControl
"Name" |> Binding.oneWay (fun mdl -> nameOf mdl)
// for Toggle UserControl only
"BoolValue" |> Binding.twoWay
(function | Toggle mdl -> mdl.Value | _ -> failwith "Wrong kind")
(fun v mdl -> ChangeValue (box v))
// for ItemChoice UserControl only
"ItemList" |> Binding.oneWay (function | ItemChoice mdl -> mdl.Items | _ -> failwith "Wrong kind")
"SelectedItem" |> Binding.twoWay
(function | ItemChoice mdl -> mdl.Value | _ -> failwith "Wrong kind")
(fun v mdl -> ChangeValue (box v))
// for Numeric UserControl only - unfinished, doesn't handle invalid value
"NumericValue" |> Binding.twoWay
(function
| Numeric mdl -> mdl.Value |> Option.map string |> Option.toObj
| _ -> failwith "Wrong kind")
(fun v mdl -> ChangeValue (box v))
]
let bindings _ _ =
[
"DefectName" |> Binding.oneWay (fun mdl -> mdl.Name)
"Characs" |> Binding.subModelSeq
(fun mdl -> mdl.Characteristics)
(fun sub -> nameOf sub)
characBindings
ChangeCharacValue
]
XAML code is inspired by this.
But I tried to write something that would be usable with my demand, and it makes me realize that it won't help for code brevity, nor really separate the concern from the various sub-models. I can't come with something as straightforward as I thought. Typically, I can't write different bindings functions for my various sub-model without wrapping them in a DU (I should have see that coming) and adding a DU there just multiply the cases.
@cmeeren : do you think this issue is worth some changes ?
I gave some further thoughts on the matter. Here is the code I wish could be written once the change is made :
[<RequireQualifiedAccess>]
module Toggle =
type Model = { Name:string ; Value: bool }
type Message = | ChangeValue of bool
let updade (ChangeValue value) model = { model with Value = value }
let bindings () =
[
"Name" |> Binding.oneWay (fun mdl -> mdl.Name)
"ValueBool" |> Binding.twoWay (fun mdl -> mdl.Value) (fun v _ -> ChangeValue v)
]
[<RequireQualifiedAccess>]
module ItemList =
type Model = { Name: string ; Value: string option ; Items: string list }
type Message = | ChangeValue of string option
let updade (ChangeValue value) model = { model with Value = value }
let bindings () =
[
"Name" |> Binding.oneWay (fun mdl -> mdl.Name)
"ItemList" |> Binding.oneWay (fun mdl -> mdl.Items)
"SelectedItem" |> Binding.twoWay (fun mdl -> mdl.Value) (fun v mdl -> ChangeValue v)
]
[<RequireQualifiedAccess>]
module Numeric =
type Model = { Name: string ; Value: decimal option }
type Message = | ChangeValue of decimal option
let updade (ChangeValue value) model = { model with Value = value }
let bindings () =
[
"Name" |> Binding.oneWay (fun mdl -> mdl.Name)
"NumericValue" |> Binding.twoWay
(fun mdl -> mdl.Value |> Option.map string |> Option.toObj)
(fun v mdl ->
match System.Decimal.TryParse(v) with
| (true, x) -> ChangeValue (Some x)
| (false,_) -> ChangeValue None )
]
module Defect =
open Elmish.WPF.Internal
type CharacType =
| Toggle of Toggle.Model
| ItemList of ItemList.Model
| Numeric of Numeric.Model
type SubMsg =
| ToggleMsg of Toggle.Message
| ItemListMsg of ItemList.Message
| NumericMsg of Numeric.Message
let nameOf = function
| Toggle mdl -> mdl.Name
| ItemList mdl -> mdl.Name
| Numeric mdl -> mdl.Name
type Model = { Name: string ; Characteristics: CharacType list }
type Message =
| ChangeValue of string * SubMsg
let updateCharac msg name charac =
if (nameOf charac) <> name then charac
else
match charac,msg with
| Toggle charac', ToggleMsg msg' -> Toggle.updade msg' charac' |> Toggle
| ItemList charac', ItemListMsg msg' -> ItemList.updade msg' charac' |> ItemList
| Numeric charac', NumericMsg msg' -> Numeric.updade msg' charac' |> Numeric
| _,_ -> failwith <| sprintf "Unexpected message %A for charac %s" msg (nameOf charac)
let update msg model =
match msg with
| ChangeValue (id, msg') ->
{ model with
Characteristics = model.Characteristics
|> List.map (fun charac -> updateCharac msg' id charac) }
let characBindings mdl =
let bindings =
match mdl with
| Toggle _ -> Toggle.bindings
| ItemList _ -> ItemList.bindings
| Numeric _ -> Numeric.bindings
("CharacType" |> Binding.oneWay (fun mdl -> (* code retrieving an enum value*) )) :: bindings
let bindings _ _ =
[
"DefectName" |> Binding.oneWay (fun mdl -> mdl.Name)
"Characs" |> Binding.subModelSeq
(fun mdl -> mdl.Characteristics)
(fun sub -> nameOf sub)
characBindings
ChangeValue
]
Here, we have various MVU module written the way we expect them, though the code is in fine a bit longer. But the problem is that characBindings
won't compile, because it can't mix BindingSpec
of various models and messages. But there could be a map function for this :
mapBindings (fromNewModel: '_model -> 'model) ->
(toNewMsg: 'msg -> '_msg) ->
(bindingSpec: BindingSpec<'model,'msg>) ->
: BindingSpec<'_model,'_msg>
So I could write the characBindings function as such :
let characBindings mdl =
let bindings =
match mdl with
| Toggle _ -> Toggle.bindings |> List.map (fun bindings -> mapBindings (fun x -> (Toggle x)) ToggleMsg bindings)
| ItemList _ -> ItemList.bindings |> List.map (fun bindings -> mapBindings (fun x -> (ItemList x)) ItemListMsg bindings)
| Numeric _ -> Numeric.bindings |> List.map (fun bindings -> mapBindings (fun x -> (Numeric x)) NumericMsg bindings)
("CharacType" |> Binding.oneWay (fun mdl -> (* code retrieving an enum value*) )) :: bindings
Well, that's still a little bit shoddy. It's not straightforward, and for the map function first param, there may be better way to pass a pattern discriminator. Some thoughts ?
Sorry, I've been super busy the last week and will be fairly busy this week also. It's in my inbox, so I won't forget it, but I might not be able to look thoroughly at this issue this week.
Generally, I prefer using well-known and general abstractions wherever possible, but I haven't looked closely enough at your example (the most recent one or the one before that) in order to determine whether this is possible. I might ask @et1975 or @2sComplement sooner or later to weigh in on whether your usage can be solved using existing Elm(ish) abstractions (feel free to take a look now if you want).
There’s no hurry, then again I can do it with current version of Elmish.WPF, it just feels a bit odd.
I'd say the primary reason it feels odd is because Elmish.WPF uses static views. With dynamic views ("proper" Elm architecture), each UI list item would be constructed according to the source data, and illegal UI states could be avoided. But with static views, things at once become more cumbersome, because we're constructing just the view data, not views directly, and illegal combinations can't be avoided generally.
Whaddayaknow, I took a brief look at your code and got thoroughly nerd-sniped into spending an hour on this after all.
Again, static views do not play well with an Elm architecture. Elmish.WPF tries to do what it can, but static views impose some fundamental constraints, so there's a limit to what can be done.
I'm very much opposed to passing the model to the bindings function, as this would allow users to use that model directly in the bindings instead of the model passed as the input parameter to each binding, which would cause bugs since the "top-level" model only reflects the initial state of the model. Furthermore, for the same reason, if a model changes from one type to another, the bindings would not get re-created and only the bindings for the previous model would ever be present.
Below I've shown more or less how I'd do it, based on your first example. I got rid of the boxing/unboxing to make it more type-safe and used twoWayIfValid
for the decimal parsing. I've also separated the parent model and sub-model with their own Model/Msg/update
, which I recommend you always do to keep things clear when you use any kind of Binding.subModel
.
I wouldn't want to throw exceptions in production just due to a mismatch between the XAML bindings and the view model. In debug it's fine because you'd want to know of a bug in your app, but in production I'd rather the app didn't crash. Hence the debugFail
method at the top which returns the default value for a type outside of debug mode.
/// Fails in debug mode, otherwise returns Unchecked.defaultOf<'a>.
let debugFail<'a> : 'a =
#if DEBUG
failwith "Should never happen"
#else
Unchecked.defaultof<'a>
#endif
let parseDecimalEmptyOk str =
if String.IsNullOrEmpty str then Ok None
else
match Decimal.TryParse str with
| true, d -> Ok (Some d)
| false, _ -> Error "Please enter a number"
[<RequireQualifiedAccess>]
module Toggle =
type Model =
{ Name: string
Value: bool }
let update value model =
{ model with Value = value }
[<RequireQualifiedAccess>]
module ItemChoice =
type Model =
{ Name: string
Value: string option
Items: string list }
let update value model =
{ model with Value = value }
[<RequireQualifiedAccess>]
module Numeric =
type Model =
{ Name: string
Value: decimal option }
let update value model =
{ model with Value = value }
module Characteristic =
type Model =
| Toggle of Toggle.Model
| ItemChoice of ItemChoice.Model
| Numeric of Numeric.Model
let getName = function
| Toggle x -> x.Name
| ItemChoice x -> x.Name
| Numeric x -> x.Name
let getType = function
| Toggle _ -> "toggle"
| ItemChoice _ -> "choice"
| Numeric _ -> "numeric"
type Msg =
| UpdateToggle of bool
| UpdateItemChoice of string option
| UpdateNumeric of decimal option
let updateMatching name msg model =
if (getName model) <> name then model
else
match model, msg with
| Toggle m, UpdateToggle x -> Toggle (Toggle.update x m)
| ItemChoice m, UpdateItemChoice x -> ItemChoice (ItemChoice.update x m)
| Numeric m, UpdateNumeric x -> Numeric (Numeric.update x m)
| _ -> model
let bindings () =
[
"CharacType" |> Binding.oneWay getType
"Name" |> Binding.oneWay getName
"BoolValue" |> Binding.twoWay
(function Toggle m -> m.Value | _ -> debugFail)
(fun v m -> UpdateToggle v)
"ItemList" |> Binding.oneWay
(function ItemChoice m -> m.Items | _ -> debugFail)
"SelectedItem" |> Binding.twoWay
(function ItemChoice m -> m.Value | _ -> debugFail)
(fun v m -> UpdateItemChoice v)
"NumericValue" |> Binding.twoWayIfValid
(function
| Numeric m -> m.Value |> Option.map string |> Option.toObj
| _ -> debugFail)
(fun v m -> parseDecimalEmptyOk v |> Result.map UpdateNumeric)
]
module Parent =
type Model =
{ Name: string
Characteristics: Characteristic.Model list }
type Msg =
| CharacteristicMsg of string * Characteristic.Msg
let update msg model =
match msg with
| CharacteristicMsg (name, msg') ->
{ model with
Characteristics =
model.Characteristics |> List.map (Characteristic.updateMatching name msg')
}
let bindings _ _ =
[
"DefectName" |> Binding.oneWay (fun mdl -> mdl.Name)
"Characs" |> Binding.subModelSeq
(fun m -> m.Characteristics)
Characteristic.getName
Characteristic.bindings
CharacteristicMsg
]
Thank you for the coding advice, the debugFail function is something I'll use more.
I disagree on the fact that passing the model is confusing. It's already the case for the main binding function and we could expect users to know better. But I admit that passing the model is not enough for managing such case, so I'll do without.
I close the issue, but still think such case deserve something more than coding around.
Do let me know if you come up with a good solution. :)
So I had my nerd-sniping moment this week-end and came with some change you'll find there (no pull-request, I forked before the commit you made lately).
The good news : it does NOT pass the model to the sub-model bindings function, so we won't have to argue over this :) The lesser good news : it takes 2 functions to create a binding :
let bindings _ _ =
let tryGetToggle = function | Toggle m -> Some m | _ -> None
let tryGetItemList = function | ItemList m -> Some m | _ -> None
//let tryGetNumeric (x:int) = None // obviously wrong. Will compile but crash at run-time.
let tryGetNumeric = function | Numeric m -> Some m | _ -> None
[
"Characs" |> Binding.multiSubModelSeq (fun m -> m.Characteristics) getName
|> Binding.addSubModel tryGetToggle Toggle.bindings ChangeToggleValue
|> Binding.addSubModel tryGetItemList ItemList.bindings ChangeItemListValue
|> Binding.addSubModel tryGetNumeric Numeric.bindings ChangeNumericValue
"Reverse" |> Binding.cmd (fun _ -> Reverse)
]
Though that point is surely improvable, the rest is pretty straightforward :
[<RequireQualifiedAccess>]
module Toggle =
type Model = { Name:string ; Value: bool }
type Message = | ChangeValue of bool
let updade (ChangeValue value) model = { model with Value = value }
let bindings () =
[
"CharacType" |> Binding.oneWay (fun _ -> "Toggle")
"Name" |> Binding.oneWay (fun mdl -> mdl.Name)
"Value" |> Binding.twoWay (fun mdl -> mdl.Value) (fun v _ -> ChangeValue v)
]
[<RequireQualifiedAccess>]
module ItemList =
type Model = { Name: string ; Value: string option ; Items: string list }
type Message = | ChangeValue of string option
let updade (ChangeValue value) model = { model with Value = value }
let bindings () =
[
"CharacType" |> Binding.oneWay (fun _ -> "ItemList")
"Name" |> Binding.oneWay (fun mdl -> mdl.Name)
"ItemList" |> Binding.oneWay (fun mdl -> mdl.Items)
"SelectedItem" |> Binding.twoWay (fun mdl -> mdl.Value |> Option.toObj ) (fun v _ -> ChangeValue <| Option.ofObj v)
]
[<RequireQualifiedAccess>]
module Numeric =
type Model = { Name: string ; Value: decimal option }
type Message = | ChangeValue of decimal option
let updade (ChangeValue value) model = { model with Value = value }
let bindings () =
[
"CharacType" |> Binding.oneWay (fun _ -> "Numeric")
"Name" |> Binding.oneWay (fun mdl -> mdl.Name)
"Value" |> Binding.twoWay
(fun mdl -> mdl.Value |> Option.map string |> Option.toObj)
(fun v mdl ->
match System.Decimal.TryParse(v) with
| (true, x) -> ChangeValue (Some x)
| (false,_) -> ChangeValue None )
]
module Defect =
type CharacType =
| Toggle of Toggle.Model
| ItemList of ItemList.Model
| Numeric of Numeric.Model
type Model =
{ Characteristics: CharacType list }
type Message =
| ChangeToggleValue of string * Toggle.Message
| ChangeItemListValue of string * ItemList.Message
| ChangeNumericValue of string * Numeric.Message
| Reverse
let init (model: Model) = model
let getName = function
| Toggle mdl -> mdl.Name
| ItemList mdl -> mdl.Name
| Numeric mdl -> mdl.Name
let update msg model =
let updateCharac msg charac =
let checkName id = id = getName charac
match msg,charac with
| ChangeToggleValue (id,msg'), Toggle sm ->
if checkName id then Toggle.updade msg' sm else sm
|> Toggle
| ChangeItemListValue (id,msg'), ItemList sm ->
if checkName id then ItemList.updade msg' sm else sm
|> ItemList
| ChangeNumericValue (id,msg'), Numeric sm ->
if checkName id then Numeric.updade msg' sm else sm
|> Numeric
| _,_ -> charac
match msg with
| ChangeToggleValue _
| ChangeItemListValue _
| ChangeNumericValue _ ->
{ model with Characteristics = model.Characteristics |> List.map (updateCharac msg) }
| Reverse ->
{ model with Characteristics = model.Characteristics |> List.rev }
let bindings _ _ =
let tryGetToggle = function | Toggle m -> Some m | _ -> None
let tryGetItemList = function | ItemList m -> Some m | _ -> None
//let tryGetNumeric (x:int) = None // obviously wrong. Will compile but crash at run-time.
let tryGetNumeric = function | Numeric m -> Some m | _ -> None
[
"Characs" |> Binding.multiSubModelSeq (fun m -> m.Characteristics) getName
|> Binding.addSubModel tryGetToggle Toggle.bindings ChangeToggleValue
|> Binding.addSubModel tryGetItemList ItemList.bindings ChangeItemListValue
|> Binding.addSubModel tryGetNumeric Numeric.bindings ChangeNumericValue
"Reverse" |> Binding.cmd (fun _ -> Reverse)
]
Let me know if it's interesting.
It's certainly interesting, and I appreciate your efforts at finding a solution to this problem.
As I am sure you have realized, static, non-composable XAML views and The Elm Architecture are in many ways at odds with each other. It works fine for simple stuff, but as you can see, no matter how we slice it, the lack of composable, dynamic views means there is no way around at least some quirks for many use-cases such as this.
Regarding your proposed solution: It's slightly more elegant in the definition of the sub-bindings, but it requires users to learn two new binding functions which work in a distinctly different way than all other bindings. It also does not provide any added type safety, because the user might forget a call to addSubModel
. Thus, I fail to see any net benefits to this method over the one I suggested further up, which I still think is the one that treads the best balance between type safety, "Elmishness", terseness, and ease-of-use: Define all bindings no matter the element, and just return the type's default value if it’s not relevant, since it won't be bound anyway. It does not require any new mental models of binding types, and with the right trivial helper functions (such as debugFail
) it’s also syntactically terse.
Thanks again for you efforts, though!
Say you have a list of items you want to display. These items can be of different types, complex enough to have distinct models, bound to different Xaml representations (the xaml code pick them through
ItemsControl
'sItemTemplate
or other ways).Those different types of item, if displayed through
Binding.subModelSeq
, must share the sameSubModelSeqSpec
, and it must contains the bindings of all possible types of item. For instance, one of your item is aDateTime
to be displayed with aDatePicker
with bindings onSelectedDate
,DisplayDateStart
andDisplayEndDate
; another is a choice displayed through aCombBox
with bindings onSelectedItem
andItemsSource
; then the bindings list has to have binding specs to all these properties, no matter that the date item don't need the list of choice or the choice item don't need the date limits.So the idea is that you can pick the right bindings list when the
Binding.subModelSeq
function is called.Its signature would change like that :
or maybe :
From there, the sub-model bindings function can pick a fitting list of binding from the sub-model type.
It breaks backward compatibility, but the changes are easy to handle for someone who don't care. Just change your sub-model bindings definition from
let bindings () = ...
tolet bindings _ = ...
A similar change could be done to
Binding.subModel
andBinding.subModelOpt
.