HeinrichApfelmus / threepenny-gui

GUI framework that uses the web browser as a display.
https://heinrichapfelmus.github.io/threepenny-gui/
Other
437 stars 77 forks source link

Why does sink return a value? #232

Closed Dark-Ethereal closed 4 years ago

Dark-Ethereal commented 5 years ago

I'm very much confused by this:

sink :: ReadWriteAttr x i o -> Behavior i -> UI x -> UI x
sink attr bi mx = do
    x <- mx
    window <- askWindow
    liftIOLater $ do
        i <- currentValue bi
        runUI window $ set' attr i x
        Reactive.onChange bi  $ \i -> runUI window $ set' attr i x
    return x

sink takes an mx... binds x to mx, then returns an x completely unchanged! It's just returning the value it was given as an argument!

What's the point in returning x at all?

In fact, come to think of it, why does sink need to take a UI action that returns an x if all it is going to do is use >>= (hidden in do notation to apply it to a function taking just x?

Why isn't it sink defined thus:

sink :: ReadWriteAttr x i o -> Behavior i -> x -> UI ()
sink attr bi x = do
  window <- askWindow
  liftIOLater $ do
      i <- currentValue bi
      runUI window $ set' attr i x
      Reactive.onChange bi  $ \i -> runUI window $ set' attr i x

We can see the same pattern on set: it's taking a UI x to make a UI x but looking at the insides, clearly it only needs a x to make a UI ()

#+ also probably needs the same treatment.

This would get rid of the need for pointlessly wrapping values with element (which is just return) just so we could have sink unwrap them inside with >>=

It makes no sense to use do notation to unwrap a UI Element so we can use it with something that takes just an Element, and then feed it to return/element to make it a UI Element just so we can feed it to something that needed a UI Element in the first place.

Yet this superflous wrapping and unwrapping is the way all the example code is structured.

ReaderT It!

I'm pretty sure the reason it's being done is so you can chain things like so:

UI.button # sink UI.text (show <$> val) # set UI.align "left"

But this is the wrong solution (IMO).

sink UI.text (show <$> val) and set UI.align "left" both need the same element to act upon...
sounds like a reader pattern.... but then they make (or should make IMO) a UI () action that needs to be combined with the >> according to UI's Monad instance...

That's a ReaderT! We could massage the definition of set and sink so they return ReaderT x UI ().

We could then define onElement = runReaderT
then

button <- UI.button # sink UI.text (show <$> val) # set UI.align "left"

becomes

button <- UI.button 
onElement button $ do
  sink UI.text (show <$> val)
  set UI.align "left"

Which is perfectly sensible: just like IORef, we're separating making the reference from the action takes the reference and makes an action that changes what it refers to.

Obviously you could take this further by hiding the use of ReaderT through newtypes or type synonyms so that sink UI.text (show <$> val) has type ElementModifier or something and onElement has type Element -> ElementModifier -> UI ()

HeinrichApfelmus commented 5 years ago

Yes, this is to allow chains with (#). The main advantage is that we can create elements and assign values to their properties without binding them to a variable. Example:

getBody #+
    [ string "Click the following"
    , UI.button # sink UI.text (show <$> val) # set UI.align "left"
    ]

Here, we do not need to give a name to the button.

The flip side is that whenever we do have a name, e.g. myButton :: Element, then we have to wrap it in element (aka return) in order to chain it.

I believe this tradeoff is worth it, but I am open to discussion. How would you formulate the example above in your approach?