Best practice for updating an incremented id in the model?

I’m building my first Elm-app, and struggle a little with how I should handle updates when I need a generated identifier for one of my types.

I have a module Label with a record type alias Label. The record contains a field of type LabelId which is a guarded Int. I also have a function to generate a new Label with some defaults. Currently it takes a LabelId and returns a tuple with the next LabelId and the new Label.

type alias Label =
    { id : LabelId, [...] }

type LabelId
    = LabelId Int

create : LabelId -> (LabelId, Label)
create id = ...

In the model I keep a List of Labels and the next LabelId:

type alias Model = 
    { nextLabelId : Label.LabelId
    , labels : List Label.Label
    }

Now in update, when I need a new Label, I could call Label.create model.nextLabelId directly, but then I must always remember to store the returned LabelId in the model. Or I could wrap Label.create with a function in Main:

createLabel : Model -> (Model, Label)
createLabel model =
    let
        ( nextNextId, label ) =
            Label.create model.nextLabelId
    in
        ( { model | nextLabelId = nextNextId }, label )

I still have to remember to use the wrapper and to use the returned model in the result of the update function, but my gut feeling is that it is slightly less risky than the first approach.

What would be a best practice for handling this? Maybe I’m making things over complicated and there is a much better approach.

I don’t really think there is a much better way. In particular there isn’t a nice way to ensure that you remember to update the model. This is one of those (rare) bugs that are more likely in an immutable language than a mutable one, similar to maintaining a random seed.

The best advice is to use code-linters, such as elm-review, this (configured correctly) will alert you if you fail to make use of a defined name. This significantly reduces the chances that you won’t update the model. You could still fail, but if, for example you have the following in your update function you will get a warning:

update ...
     case message of
          ...
          NewLabel ->
                 let
                        (newModel, newLabel) =
                                createLabel model
                        ....
                 in
                 ( { model | labels = ... }
                 , Cmd.none
                 )
      ...

Here at the bottom I’ve accidentally updated the original model rather than newModel. That’s a bug, but the static analysis tool will warn you that newModel is defined but unused. You can of course still mess up, but it’s less likely, you might do:

                 let
                        (_, newLabel) =
                                createLabel model
                        ....
                 in

or

                 let
                        newLabel =
                                createLabel model
                                       |> Tuple.second
                        ....
                 in

So it’s still possible to introduce a bug this way.

Note, I think if you really want to ensure that this doesn’t happen, you could do define your model as an opaque type (a custom tagged union typed whose module does not export its constructors). So you can do something like the following:

module Module exposing (State, Id, Model, update, updateWithNewId)

type alias Id =
    String

type alias State a =
    { things : List Thing
    , ...
    }
type Model =
    Model (State { idsSoFar : Int} )

update : (State a -> State a) -> Model -> Model
update updateState model =
    case model of
        Model state ->
            Model (updateState state)

updateWithNewId : (Id -> State a -> State a) -> Model -> Model
updateWithNewId  updateState model =
    case model of
        Model state ->
            let
                newId =
                    String.fromInt state.idsSoFar
                        |> String.append "id-number-"
                newState =
                    { state | idsSoFar = state.idsSoFar + 1 }
            in
            Model (updateState newId newState)

So basically State here would be the entire rest of your model. You could also, if you like, make the type of Id an opaque type so that it is not possible create one without going through this model. This is not pretty. Your update function would look something like:

update message model =
     case message of
           Tick now ->
               ( Model.update (\s -> { s | now = now }) model
               , Cmd.none
               )
           ....
           NewThing ->
              let
                    updateFun newId state =
                         { state | things = Thing.empty newId :: model.things }
              in
              ( Model.updateWithNewId updateFun model
              , Cmd.none
              )
           ...

You could probably make this a little more palatable by separating out your messages into those that require a new Id and those that do not and then just matching within those.

1 Like

It’s also worth asking if you need auto-incrementing ids in the first place. Here are some alternatives:

Position

If you just need a way to refer to an item when multiple are interactable on the screen, sometimes position in a list works well enough

e.g.

in the view:

List.indexedMap (\idx _ ->
  button [ onClick (Toggle idx) ] [ text "Toggle Item" ]
) items

Then in the update:

Toggle target ->
  List.indexedMap (\idx item ->
    if idx == target then toggle item else item
  ) items

Another existing unique attribute

Sometimes you get lucky and the item already has another property that’s unique that you can use as a lookup key.

Generated on the backend

If you are persisting values to a backend, then it will probably want to manage generating ids. You may need to keep track of the distinction between persisted items that have an id and non-persisted items that don’t.

Locally-generated UUID

Another alternative is to generate a UUID locally. This requires randomness and can be done either as part of a command, or synchronously by providing a seed. There are several packages that provide UUID generators.

2 Likes

Thank you for your answer. It’s always a bit terrifying to expose ones lack of knowledge or insight, but you made me feel my question was reasonable.

I have not been using elm-review, (only elm-format which I find incredibly helpful), so introducing that seems like a good idea to guard me against myself. I can probably live with some uncertainty - for now that seems better for me than the added complexity of your other solution.

Thanks also for the link to that blog. It’s really valuable to get some good sources of inspiration and guidance when you enter a new community.

1 Like

Thank you joelq for your suggestions. I have been contemplating the alternatives. Initially I thought it would be hard to achieve what I’m doing with any of them except for the UUIDs, which I believe would need the same consideration as my current incrementing Int only with a seed instead?

I use the ID:s in three ways:

  1. To track user interaction with labels in my list
  2. To track the fetching of data for the labels through HTTP
  3. To track the labels through JS interop, passing the id in the outgoing port and reading it back in the incoming port. (I’m printing, not persisting.)

My list is really a history of label-printing-attempts, where I track their current state. The same data can be used for several attempts, with different (or the same) resulting states depending on external factors, and that should be shown in a linear history. So there is not any natural unique key.

The asynchronous nature of my use cases had me believe an index would not work. Until I realised I do not alter the list much, given its linear nature. It is only ever appended to (currently prepended, but that would have to change) and the only way indexes change is on removal. But since this history is session based, there is no real concern if removal just means hiding. That might just work, and hopefully reduce the complexity of my code a bit. I’ll probably look into this.

Thanks again for helping me question my assumptions.

@Daniel very valid question, and your solutions and intuitions are right.

The previous comments have very valid solutions and questions, with different tradeoffs. Another one, in the same vein of:

Would be to make a Labels data structure, which has an opaque type in its own module with the ids and labels, and the functions to manipulate labels.

Inside the module you can keep consistency and not expose Labels or Label internals in a way that would make this inconsistent.

I think one important question is what are you doing with a Label when you create it in your update function.

If what you do to the newly created Label is modify it based on some logic, but don’t need the Label info for other things, you could pass a lambda to the createLabel function allowing to modify the Label before storing it. So you could end up with a nice module with something like this (just the types for brevity):

module Labels exposing (Labels, Id, empty, createLabel, map, view)

import Label

type Id

type Labels = Labels

empty : Labels

createLabel : (Label -> Label) -> Labels -> Labels

map : (Label -> Label) -> Id -> Labels -> Labels

-- For event handlers or other configuration
type ViewOptions

view : ViewOptions -> Labels -> Html msg

When using it from your update function, you don’t have access to the internals of Labels so you can’t mess it up. You could forget to update Labels into the model but you wouldn’t see the new label either, which is more obvious.

update msg model =
    case msg of
        NewLabel ->
            ( { model | labels = Labels.createLabel (Label.updateText "Banana") }
            , Cmd.none
            )

This of course depending on what you may be doing may be overkill, and I think your solution of having a top level function to create a label is a perfectly reasonable option with different tradeoffs.

Thank you, @joakin! This was a really interesting approach.

I had already been wondering if my helper functions in Main called for a custom type for the list of labels in my Model, but was reluctant to introduce my own collection like type. That I could also move the responsibility to manage the next Id into this type had not occurred to me.

I took the bait and refactored this out, much like the types and function signatures you suggested. It turned out very nicely.* It reduced so much code at the call sites that it almost compensated for the overhead of the module in terms of LoC, and the remaining code is much clearer.

I think I will also want to make some helpers in the Label module for updating individual fields, like your Label.updateText example, to get rid of some of the anonymous functions I now use.

I will mark your answer as the solution as that is what I used, but the suggestions from both allanderek and joelq were solid advice as well!

* This was my first experience of refactoring in Elm. That too turned out very nicely. I wrote the new module in cooperation with elm-format (and of course your code suggestions), changed the type in the model to use my new History type, chased downed the compiler errors one by one and replaced the offending code with new code using functions from the module, and then tested in my browser. Everything worked. This is no news to you, but it is such a wonderfully different experience from the other languages I use that I just have to point it out.

2 Likes

If you have the time, "Make Data Structures" by Richard Feldman - YouTube is a great talk, and for me personally one of the most influential ones that helped me shape how I write Elm code :smiley:

2 Likes