How I Reduced My Code Size by 800%

Show and Tell
1

TL;DR - I eliminated a bunch of update code by moving it into the view. It was a good idea. Trust me.

I have an application that manipulates lists quite a bit. The user can add things at the beginning, end, move things around, etc. Each item in the list is one of a few different element types, each with their own set of input fields. I implemented this a long time ago, when I was very new to Elm, in the naive and obvious way - msg types for every field of each element type, and msgs for all the list manipulation operations. There were a lot of them.

This was a painful file of code that was very difficult to understand if only for its sheer size. Something like 700 lines of case statements doing record updates and handing Maybe cases that could never happen. (Like the UpdateElement Int UpdateElementMsg which needed to account for the element at the given index not existing, or not being the type the UpdateElementMsg was for - neither of which would ever actually happen.)

So I tried to fix it now that I have much more experience with Elm. I ended up deflating 700 lines to around 80. The code for the entire list editing webpage nearly fits in my editor’s minimap now, which is neat. I figured I share the two techniques I used - maybe they’ll help someone else.

The first thing was to switch the view functions for each element type from viewElement : Element -> Html Msg to viewElement : Element -> Html Element. This added a few \input -> { element | text = input } lines to each element view function, but eliminated a ton of Msg types and update case statements.

So that cleared up most of the Element updating code, but the list manipulation code was still an extra-large plate of spaghetti. So I wrote this function:

viewList :
    { onSet : List data -> msg
    , viewItem :
        { item : data
        , set : data -> msg
        , delete : msg
        , insertAfter : data -> msg
        , insertBefore : data -> msg
        , moveBackward : msg
        , moveForward : msg
        }
        -> viewItemResult
    , viewList :
        { items : List viewItemResult
        , append : data -> msg
        , prepend : data -> msg
        }
        -> viewResult
    }
    -> List data
    -> viewResult
viewList config list =
    config.viewList
        { append = List.singleton >> (++) list >> config.onSet
        , prepend = (::) >> (|>) list >> config.onSet
        , items =
            List.indexedMap
                (\index item ->
                    config.viewItem
                        { item = item
                        , set = List.Extra.setAt index >> (|>) list >> config.onSet
                        , insertAfter = insertAt (index + 1) >> (|>) list >> config.onSet
                        , insertBefore = insertAt (index + 1) >> (|>) list >> config.onSet
                        , delete = config.onSet (List.Extra.removeAt index list)
                        , moveBackward = config.onSet (List.Extra.swapAt index (index - 1) list)
                        , moveForward = config.onSet (List.Extra.swapAt index (index + 1) list)
                        }
                )
                list
        }

That function does in 18 lines what used to take about 200. (I know what you’re thinking. “(::) >> (|>) list >> config.onSet. Really?” I swear it’s totally easy to read after understanding the basic pattern of >> (|>) x. Replace it with a lambda if you want.)

Now I can write the list view function like this (where the viewUnit function has the type Unit -> Html Unit):

type Unit
    = Text { content : String }
    | Audio { url : String }
    | Video { url : String }

view =
    viewList
        { onSet = SetList
        , viewItem =
            \{item, moveBackward, moveForward, delete, set} ->
                Element.tile
                    { header =
                        Element.horizontal
                            [ Element.upChevron moveBackward
                            , Element.downChevron moveForward
                            , Element.deleteIcon delete
                            ]
                    , body =
                        [ Html.map set (viewUnit item)
                        , ... some controls for inserting elements above and below ....
                        ]
                    }
        , viewList =
            \{ items, append } ->
                Element.vertical (
                    items
                    ++
                    [ Element.button "Add Text" (append (Text { content = "" }))
                    , Element.button "Add Audio" (append (Audio { url = "" }))
                    , Element.button "Add Video" (append (Video { url = "" }))
                    ]
                )
        }

My main takeaway from this exercise is “Msg types can be whatever the heck you want them to be, and probably should be.” (Obligatory incantation to keep the best-practice gnomes happy: Except functions. Never put functions in your messages.)

P.S. I might have code-golfed a bit too much with that viewList function. At least it was fun to write.

5 Likes
2

Interesting! Can you show what your update function looks like now, too?

3

As far as I remember some time ago there were chances of getting race conditions, if you update your model in your view. Not sure if it still applies, but I’d personally double check :slight_smile:

1 Like
4

Yes viewElement : Element -> Html Element can lead to race conditions.
If you for example have both an onBlur and an onClick event which fire at the same time, and one returns {element | foo = True} and the other returns {element | bar = True}, then one will overwrite the other, so that only either foo or bar will be True.

I think to avoid that you could make it viewElement : Element -> Html (Element -> Element) instead. So that the message is a function, which gets supplied with the element in the update function.

1 Like
5

Actually it rarely seems to happen with onBlur + onClick, unless you have a supersonic finger, because onBlur fires when you press the button, and onClick fires when you release it.

Here’s a repro with onBlur + onMouseDown though:
https://ellie-app.com/7SP32mLcqFZa1
Click on the input field, then click on the button. What you might expect is for both foo and bar to change, but only one of them does.

(onBlur + onMouseDown) is just one example, this could potentially happen with any event.

1 Like
6

This goes agains the recommendation against sending functions in messages.

I would be curious what other events can trigger this behavior when combined (outside of focus events).

7

Basically just this:

update msg model =
    case msg of
        SetList newList ->
            ({model | list = newList}, Cmd.none)
        ...
8

This is why I limit the use of Html Element. There are lots of other msgs that the various parts of the page can produce - but the Html Element type is kept to as small of a portion of the view as I can.

Limiting the use of Html Element is also one of the reasons the viewList function does the weird twist-around thing with the msg types to create a data -> msg function to pass to the provided view - so that the function you give it can produce Html Msg instead of Html Element - that way you can produce all the other more custom Msg types and keep the Html Element as targeted as possible, reducing the chance of race conditions.

In this case the only events that produce Html Element are onInput for input fields and onClick for checkbox fields. So no chance of race conditions as far as I can tell.

It’s also enforced with my form library which looks like this:

Form.view
    [ Form.text "Video Title" Focus.title
    , Form.errors [(TitleEmpty, "The title is required")]
    , Form.text "Video URL" Focus.url
    , Form.errors [(UrlEmpty, "The URL is required"), (UrlInvalid, "That doesn't look like a URL!")]
    ]

If you stick to the Form.view function, race conditions like that can’t happen because it only attaches onInput handlers to the dom.

1 Like
9

I’ll admit that I’ve never seen f >> (|>) x before, and it wasn’t immediately obvious to me what it means, but eventually I worked out that it’s equivalent to flip f x where flip exchanges the first and second arguments of f (flip : (a -> b -> c) -> (b -> a -> c)). So in this case,

(::) : a -> List a -> List a
and
(::) >> (|>) list = flip (::) list = (\item -> item :: list)

I think I find flip (::) list to be a good compromise between clarity and conciseness.

1 Like
10

My thought process went: “I’d just use flip here. But Elm doesn’t have flip built-in anymore, does it? Guess I could just use a lambda, or write flip myself. The value will be a function without the flip, right? Can I just apply it in the pipeline? Oh, I can. Neat. I probably shouldn’t write this.” I then proceeded to write it anyway.

1 Like
11

My first exposure to any kind of functional programming was JavaScript’s array functions map(), filter(), and reduce(). I started using them almost any time I could, chaining them together and usually using lambdas. Looking at my code from back then, you’d think I was trying to see how much I could accomplish in a single statement, to the detriment of readability.

These days, I’m trying to pay more attention to whether the code is clear to the reader, even though the expressiveness afforded by functional programming styles can be addicting!

12

I think writing yourself flip or importing from basics-extra will be much clearer

13

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.