I've watched "the life of a file" but my Main.elm feels too large and messy

Request Feedback
1

Hello all

Those of you on Slack will have mostly likely seen my project. It is deployed and the code is all open source.

My app is a lyrics archive. You search for an artist, you see lyrics for that artist. You click a lyric to read it. Simple (so far at least). The app is geared towards mobile devices, so if the layout looks weird, it is probably because you’re viewing it on a desktop.

I have watched The Life of a File talk by Evan and it all makes sense. But I feel like I’ve gone beyond that and I should really start to split things out, I’m just not sure how.

Also, certain things feel wrong to me, I find myself doing case apiRootUrl of a lot.

I would appreciate some constructive feedback on my code. I feel like I’m doing things badly.

Thank you.

PS. Feel free to chat to me on Slack if you have questions on the state of things.

2

For the apiRootUrl. What you can do is to use a json decoder in init. If they fail to decode then you can show an error state for the app. If it decodes then apiRootUrl will be already a record.

Something like:

init : Decode.Value -> Url -> Nav.Key -> ( Model, Cmd Msg )
init jsonFlags url key =
  case Decode.decodeValue flagsDecoder jsonFlags of
    Err err ->
       (some error view here, some command to log this)

    Ok flags ->
      case parsedUrl of
          .... flags.apiRootUrl is already parsed
1 Like
3

Instead of rewriting case remoteData of every time you can make a function like this:

viewData : WebData (Html msg) -> Html msg
viewData data =
    case data of
        NotAsked ->
            text ""

        Loading ->
            showLoader

        Failure _ ->
            showError

        Success html ->
            html

You can then rewrite your view functions like this:

viewLyric : Lyric -> Html Msg
viewLyric lyric =
    p [ class "lyric__body" ] [ text lyric.body ]

And then you call them like this instead:

viewData (RemoteData.map viewLyric lyric )

If you have a view function which depends on two or more pieces of RemoteData, you can use RemoteData.map2 etc.

I would move all types like Artist, Lyric and their decoders etc. out of Main.elm

Another minor thing is I wouldn’t define any “list decoders”, just define the decoder for the inner type, and use Decode.list inline.

4

This is hints toward improper representation. In this particular case, apiRootUrl has no business being a Maybe. If that Url.fromString evaluates to a Nothing the app should not start. You can have a FatalError page if you want and that page could be used for such contexts. The alternative would be to make your app unresponsive by using a default url.

This is a matter of personal preferences at this point. If you want inspiration, check the elm-spa-example. That app presents a lot of good practices. Richard is one of the core team members and he has spent a lot of time thinking about the layout of the files and the splitting of that app.

The way I would split it is different and more in line with the original version of elm-spa-example (the current version is the second incarnation).

The way I would split your code would be like this:

  1. Create a Data.elm file and move there the business objects (Artist, Lyric) and their decoders.
  2. Change the Endpoint.elm to Api.elm and move all the api calls there. The functions exposed by this module should take in a Context record (where I would put the apiRootUrl and the Key), some kind of arguments (e.g. search string) and a toMsg: Result Http.Error SomePayload -> msg function that would allow you to have each function return Cmd msg. This module would import from Data. This would be the only module importing Http.
  3. Create a Route.elm that would hold the Route custom data type and all the functionality around routing. I would only use the Nav module functions in this module and expose an API that deals with Route values instead of raw Url.

These things should already clean your code considerably. Moving on, as the project develops, I would have a Ui.elm with all kinds of small view functions (the equivalent of components from other languages). Most likely a Pages folder would be the next split and move there, the code of each page (model, update, view) on a one page = one module structure.

As the project grows, it might be required to split modules by sections. So, Api.elm could keep some of the more used functions but a new Api folder could be created next to it and move there code dealing with only a particular section e.g. module Api.Artist, module Api.Lyrics. This would be an overkill at the current stage. The same with Data.elm, you could have a Data folder with module Data.Artist but it would again be an overkill in the current scenario. (I do have one of these Data files that already has some 800 LOC). The Ui.elm could also get a Ui folder for very complex widgets.

2 Likes
5

In addition to the things others have said, Richard Feldman gave a talk the next year (if I recall correctly) that gives more advice on how to split out data structures:

1 Like
6

Thanks so much. Can you clarify something please.

When you said to move all the api calls into Api.elm, do you mean the actual functions that make the request, like the below?

searchArtists : String -> String -> Cmd Msg
searchArtists apiRootUrl searchTerm =
    let
        endpoint =
            searchArtistsEndpoint apiRootUrl searchTerm
    in
    request
        { method = "GET"
        , headers = []
        , url = endpoint
        , body = Http.emptyBody
        , expect = Http.expectJson ArtistsRetrieved artistListDecoder
        , timeout = Nothing
        , tracker = Nothing
        }
7

Yes, they will become something like this:

searchArtists : String -> String -> (Result Error (List Artist)) -> Cmd Msg
searchArtists apiRootUrl searchTerm toMsg =
    let
        endpoint =
            searchArtistsEndpoint apiRootUrl searchTerm
    in
    request
        { method = "GET"
        , headers = []
        , url = endpoint
        , body = Http.emptyBody
        , expect = Http.expectJson toMsg artistListDecoder
        , timeout = Nothing
        , tracker = Nothing
        }

The main idea is to isolate the complexity related to http calls and decoding away from the place you will actually use it. Another way to think about it is to view Api.elm as a higher level http module, a module that understands and speaks in your apps data structures.

3 Likes
8

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