tiziano88 / elm-protobuf

protobuf plugin for elm
https://package.elm-lang.org/packages/tiziano88/elm-protobuf/latest/
MIT License
94 stars 28 forks source link

Update Functions #18

Closed dmattia closed 6 years ago

dmattia commented 6 years ago

I am wondering your thoughts on creating chainable updateX methods for each message. This is something I often do in elm for types I create.

There is a history of such methods on proto objects:

Example:

message Model {
  string string_field = 1;
  int64 long_field = 2;
}

Example Proto Output (only new functions shown):

updateStringField : String -> Model -> Model
updateStringField newValue record =
  { record | stringField = newValue }

updateLongField : Int -> Model -> Model
updateLongField newValue record =
  { record | longField = newValue }

Example Usage:

update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
  case msg of
    SomeMsg someStr someLong ->
      ( model
          |> updateStringField someStr
          |> updateLongField someLong
      , Cmd.none )

I would gladly offer to help with this if you're interested in the idea

tiziano88 commented 6 years ago

Thanks for offering your advice and willing to help @dmattia ! I am not sure whether introducing additional combinators for this kind of updates is any more readable than just using the plain record update syntax.

In your example, the "plain" version would look like

update : Msg -> Model -> ( Model, Cmd Msg )
update msg model =
  case msg of
    SomeMsg someStr someLong ->
      ( { model
        | stringField = someStr
        , longField = someLong
        }
      , Cmd.none )

which seems pretty readable to me?

dmattia commented 6 years ago

It does to me as well. The bigger issue that I am hoping to help with is updating nested records, where helper functions can make code considerably more concise and readable (subjective).

I really like the asXIn syntax alongside the setX methods, as described here: https://medium.com/elm-shorts/updating-nested-records-in-elm-15d162e80480

tiziano88 commented 6 years ago

I feel the same about nested records, and I wish elm had something like the lens package in Haskell for that. The closest I could find is https://github.com/bChiquet/elm-accessors , perhaps we could add support for generating those combinators in proto, but AFAICT the elm community (or at least @evancz) seems to discourage that pattern (see the message when typing lens into https://package.elm-lang.org/).

dmattia commented 6 years ago

That's fair. I'm convinced enough that this isn't needed for now then.