Elmish.wpf: Advice regarding split between functions and messages

[Evangelink]

I am not sure to correctly understand when to have a message calling another message or when to call a function.

Let's take an example with a video player. I want to allow to move to previous/next frame but also a jump to whatever frame.

Assuming I have the following messages:

type Msg =
    | PreviousFrame
    | NextFrame
    | SelectFrame of int

Would you do this

let update msg model =
    match msg with
    | PreviousFrame ->
        // Some logic
        model, Cmd.ofMsg (SelectFrame newIndex)
    | NextFrame ->
        // Some logic
        model, Cmd.ofMsg (SelectFrame newIndex)
    | SelectFrame index ->
        // Some logic

or this

let selectFrame index = // some logic

let update msg model =
    match msg with
    | PreviousFrame ->
        // Some logic
        selectFrame xxx
        model, Cmd.none
    | NextFrame ->
        // Some logic
        selectFrame xxx
        model, Cmd.none
    | SelectFrame index ->
        // Some logic

or even calling Cmd.ofFunc.XXX instead of the local call to the function.

So far I am relying a lot on option 1 but I have the feeling that's the wrong option because it requires going through the update loop again, also I am trying to implement some undo/redo and it seems much more complex than if I go for option 2.

[TysonMN]

I am trying to implement some undo/redo and it seems much more complex than if I go for option 2.

I implemented some code to help with undo/redo. I link to it from issue #195. I am using in my application at work on multiple screens. You can comment there if you have any questions about it.

I am not sure to correctly understand when to have a message calling another message or when to call a function.

In general, you don't want the implementation for update in the case of a certain message to be handled by dispatching a sibling message. Nor do you want to write code like this

    | PreviousFrame ->
        // Some logic
        selectFrame xxx
        model, Cmd.none

because selectFrame is impure. It should look more like

    | PreviousFrame ->
        // Some logic
        let model = { model with CurrentFrame = selectFrame (model.CurrentFrame - 1) }
        model, Cmd.none

So far I am relying a lot on option 1 but I have the feeling that's the wrong option because it requires going through the update loop again

Indeed. The main reason option 1 is worse than option 2 (after option two is made pure) is because it is needlessly more complicated. Other less obvious reason is that you might have introduced a race condition. The value newIndex in

    | PreviousFrame ->
        // Some logic
        model, Cmd.ofMsg (SelectFrame newIndex)

is the correct index now to move to the previous frame (by assumption), but this might not be the case after that message gets to the front of the message queue again. For example, if you dispatch two PreviousFrame messages in a row, then both will be converted into SelectFrame messages with the same value of newIndex. After they are both processed, the frame index will have only decreased by one instead of by two.

[Evangelink]

because selectFrame is impure. It should look more like

I stripped too much of my code but I was thinking about this :)

I implemented some code to help with undo/redo. I link to it from issue #195. I am using in my application at work on multiple screens. You can comment there if you have any questions about it.

Thanks! I will have a look.

. Other less obvious reason is that you might have introduced a race condition.

Interesting, I didn't think about this. I do have a CurrentFrameIndex on my model updated by the SelectFrame message but I didn't think that I could get the 2nd PreviousFrame called before the actual end of the 1st, resulting in a wrong last frame being displayed.

If that's ok I will keep this open a couple of days just in case I have some other question related.

[Evangelink]

I have one more question, I created some specific messages for all impure functions. Would you recommend to keep calling them after my pure code or would you wrap them through Cmd.ofFunc.XXX?

Example with Cmd.ofMsg:

    | PreviousFrame ->
        // Some logic
        let model = { model with CurrentFrame = selectFrame (model.CurrentFrame - 1) }
        model, Cmd.ofMsg (UIMutationMsg Redraw)

Example with Cmd.ofFunc:

    | PreviousFrame ->
        // Some logic
        let model = { model with CurrentFrame = selectFrame (model.CurrentFrame - 1) }
        model, Cmd.ofFunc.perform redraw model (fun _ -> NoOp)

Just to give a little more context, the impure parts are not only impure but also not easily mockable so I would really like to be able to test the "core" part and have integration tests for the other part.

[TysonMN]

the impure parts are not only impure but also not easily mockable so I would really like to be able to test the "core" part and have integration tests for the other part.

If your impure parts are easily mockable, then it should also be easy to separate the pure parts from the impure parts.

Before I became a functional programmer, I read most of The Art of Unit Testing by Roy Osherove. Within the testing world, there are is much terminology. Roy tries to simplify things by reducing down to three term: stub and mock, both of which are a fake. After I started my journey into functional programming, I realized that mocks are a compromise--a solution to a problem that shouldn't exist. Mocks are only needed if the system under test is impure. If the system under test is pure, then stubs suffice.

The goal is to push impure behavior to the boundary of your system. Collect the pure code in the center. Then most of your tests will test pure code because most of your code is pure. You can have a few tests that test the impure code, which is enough because very little of your code is impure.

I really like Functional Programming in C# by Enrico Buonanno. In section 5.5.3, he address how to compose different sections of code in a functional way. He is essentially saying the same thing that I am. The advantage of that section is that it also includes three figures that help explain the idea. I really like those figures. One day I want write a blog post on this idea and include my own take on those figures.

I created some specific messages for all impure functions. Would you recommend to keep calling them after my pure code or would you wrap them through Cmd.ofFunc.XXX?

I prefer to use the CmdMsg pattern. By having specific messages for all of your impure behavior, it sounds like you are nearly there.

[Evangelink]

The goal is to push impure behavior to the boundary of your system. Collect the pure code in the center. Then most of your tests will test pure code because most of your code is pure. You can have a few tests that test the impure code, which is enough because very little of your code is impure.

Absolutely agree! Yet it's a bit problematic when using impure code from third-parties. I have tried to apply this as much as I can, yet there are 2 things I didn't manage to "convert":

1. I use a timer for the playback and both start and stop messages change a boolean value of my model AND call Start or Stop functions of the timer (couldn't find how to move away from this).
2. All my messages changing the image displayed have to call some logic to redraw the image which involves again call to a third-party.

I prefer to use the CmdMsg pattern. By having specific messages for all of your impure behavior, it sounds like you are nearly there.

Yep looks like this is what I need :)

Thank you for the help, it's really appreciated.

[TysonMN]

2. All my messages changing the image displayed have to call some logic to redraw the image which involves again call to a third-party.

Yes, I am doing that as well. I am using SkiaSharp in my application at work for a 2D drawing editor. To redraw, I call UIElement.InvalidateVisual on the SKElement. I pass the entire SKElement from C# to F# (because some of the drawing / binding code wants to know how large it is) when the application starts and the flow of control is passed to Elmish.WPF. Then InvalidateVisual is passed as a function through several layers of (what I call) bindCmd functions that map my custom CmdMsg DUs to Elmish.Cmd<_>s. Here is the code where InvalidateVisual is turned into an Elmish command.

let bindCmd draw = function
  | Draw -> Cmd.OfFunc.attempt draw () CmdException

Executing InvalidateVisual causes the SKElement.PaintSurface event to be raised. I convert that event into a command as shown in the EventBindingsAndBehaviors sample. The binding for that command is where I put my impure behavior. Here is the core logic in that binding.

[<RequireQualifiedAccess>]
module Draw =

  let onCanvas (canvas: SKCanvas) drawables =
    Color.white |> canvas.Clear
    drawables |> Seq.iter (Drawable.draw canvas)
    Pure.DrawScreen.NoOp

Each command binding has to return a message to be dispatched. I am currently sending a NoOp message, which of course returns the model it is given. Maybe we could adjust the interface of the CmdParam binding to support returning an optional message. In the future, I might return a message that indicates at what time the screen was drawn as part of an implementation that allows for animations or transitions.

The SKCanvas is obtained from the parameter in a CmdParam binding. I feel like this is the correct place to execute the impure behavior. Further delaying the execution of the impure behavior by sending it to Elmish as a Cmd for execution feels wrong because this is an event with a parameter that is mutated. I think the mutation should occur close in time to when the event was raised and close in space to where the mutable object was obtained.

1. I use a timer for the playback and both start and stop messages change a boolean value of my model AND call Start or Stop functions of the timer (couldn't find how to move away from this).

Maybe split this into four separate messages.

let update msg m =
  | StartPlayback -> m, StartPlaybackCmd
  | PlaybackStarted -> { m with IsPlaying = true }, Cmd.none
  | StopPlayback -> m, StopPlaybackCmd
  | PlaybackStopped -> { m with IsPlaying = false }, Cmd.none

let bindCmd = function
  | StartPlaybackCmd -> // start timer and return PlaybackStarted message
  | StopPlaybackCmd  -> // stop  timer and return PlaybackStopped message

[Evangelink]

I am closing this as I have successfully converted my code to use the CmdMsg pattern and I am really happy with the result. I have finally an entirely pure update function and I have managed to get rid of the things I wanted from my model.