Search code examples
f#code-readabilityfable-f#elmish

Avoiding code duplication in F# when making a form based app for multiple data types


I am currently making an app in F# with fable elmish architecture where the record types are as follows (cut down to save space but you hopefully get the idea).

type NewOriginMerdEntry =
    | AddOriginMerdID of string
    | AddMerdNumber of int
    | AddAverageWeight of float
    | AddPD of int

type NewTreatmentEntry =
    | AddTreatmentID of string

type NewDestMerdEntry =
    | AddDestMerdID of string

 ....etc

Now I have compiled these into a discriminate union type such as this

type NewEntry =
    | NewOriginMerdEntry of NewOriginMerdEntry
    | NewTreatmentEntry of NewTreatmentEntry
    | NewDestMerdEntry of NewDestMerdEntry
    ...etc

with finally the main message type looking like this:

type Msg = {
     NewEntry of NewEntry
}

Which is clean enough however in the view function I need to create a new function for each type symbolizing a view and the specific messages that need to be dispatched when the text input changes.

Which is something like this:

let originMerdView (dispatch : Msg -> unit) (model : Model) =
    let dispatch' = NewOriginMerdEntry >> NewEntry >> dispatch
    let form = match model.form with
                | OriginMerd o -> o
                | _ -> None

    R.scrollView[
        P.ViewProperties.Style [
            P.FlexStyle.FlexGrow 1.
            P.BackgroundColor "#000000"
        ]
    ][
        //these functions are simply calls to various input text boxes
        inputText "ID" AddOriginMerdID dispatch'
        numinputText "MerdNumber" AddMerdNumber dispatch'
        floatinputText "average Weight" AddAverageWeight dispatch'
        numinputText "PD" AddPD dispatch'
        button "save" form SaveOriginMerd (SaveEntry >> dispatch)
    ]


let inputText label msg dispatch =


    R.textInput[

        P.TextInput.OnChangeText ( msg >> dispatch )
    ]

So first question is, would it be possible to somehow generalize this as the mainview will decide which of these functions to run based on the model state. It works fine but the amount of code repetition is fairly painful.

Also each new entry will get sent to this function:

let handleNewEntry (model : Model) (entry : NewEntry) =
    match entry with
    | NewOriginMerdEntry e -> handleNewOriginMerdEntry model e
    ... etc


let handleNewOriginMerdEntry (model : Model) (entry : NewOriginMerdEntry) =
    let form =
        match model.form with
        | OriginMerd o -> match o with
                            | Some f -> f
                            | None -> OriginMerd.New
        | _ -> failwithf "expected origin type got something else handleNewOriginMerd"

    let entry =
        match entry with
        | AddOriginMerdID i -> {form with originMerdID = i}
        | AddMerdNumber n -> {form with merdNumber = n}
        | AddPD p -> {form with pD = p}
        | AddAverageWeight w -> {form with averageWeight = w}

    {model with form = OriginMerd (Some entry)}, Cmd.none

All the specific handle new entry functions are the exact same except with the different records obviously. This functions fine but again the code reuse is very painful. Is there some more elegant way of achieving the same result with less code repetition?


Solution

  • Seems to me that this part, at least, of your view is going to be shared:

    let form = match model.form with
               | OriginMerd o -> o  // With a different match target each time
               | _ -> None
    
    R.scrollView[
        P.ViewProperties.Style [
            P.FlexStyle.FlexGrow 1.
            P.BackgroundColor "#000000"
        ]
    ]
    

    I think you could pull that out into its own function, and just have the list of input fields be different. And, crucially, pass parts of your model around to those functions. E.g.,

    let originMerdForm (dispatch : Msg -> unit) (OriginMerd form) =
        let dispatch' = NewOriginMerdEntry >> NewEntry >> dispatch
        [
            //these functions are simply calls to various input text boxes
            inputText "ID" AddOriginMerdID dispatch'
            numinputText "MerdNumber" AddMerdNumber dispatch'
            floatinputText "average Weight" AddAverageWeight dispatch'
            numinputText "PD" AddPD dispatch'
            button "save" form SaveOriginMerd (SaveEntry >> dispatch)
        ]
    
    let destMerdForm (dispatch : Msg -> unit) (DestMerd form) =
        let dispatch' = NewDestMerdEntry >> NewEntry >> dispatch
        [
            inputText "ID" AddDestMerdID dispatch'
            button "save" form SaveDestMerd (SaveEntry >> dispatch)
        ]
    
    let getFormFields (model : Model) =
        match model.form with
        | OriginMerd _ -> originMerdForm model.form
        | DestMerd _ -> destMerdForm model.form
        // etc.
        | _ -> []
    
    let commonView (dispatch : Msg -> unit) (model : Model) =
        R.scrollView[
            P.ViewProperties.Style [
                P.FlexStyle.FlexGrow 1.
                P.BackgroundColor "#000000"
            ]
        ] (getFormFields model)
    

    Note that the way I wrote this, you'll get an "incomplete match case" warning on the OriginMerd form and DestMerd form parts of the respective functions. What I really wanted was to have them have the type of the entry (the o in your OriginMerd o line of your original code), but I don't know what you named that. The only change that would need to happen then is that you'd want to extract the button call to the common view, e.g.

    let commonView (dispatch : Msg -> unit) (model : Model) =
        let formFields, saveMsg = getFormFields model
        R.scrollView[
            P.ViewProperties.Style [
                P.FlexStyle.FlexGrow 1.
                P.BackgroundColor "#000000"
            ]
        ] (formFields @ [button "save" model.form saveMsg (SaveEntry >> dispatch))
    

    and then your originMerdForm, destMerdForm would return a tuple of (form fields, msg) where msg would be SaveOriginMerd, SaveDestMerd, and so on.

    Your handleNewFooEntry functions could also benefit from a similar change in input parameters: instead of passing in the whole model, you can pass in just the appropriate entry type (and rename your entry parameter to msg, please, so you don't confuse yourself). I.e., it would look something like this:

    let handleNewEntry (model : Model) (msg : NewEntry) =
        let form' =
            match msg, model.form with
            | NewOriginMerdEntry m, OriginMerd o -> handleNewOriginMerdEntry o m
            | NewOriginMerdEntry m, _ -> failwithf "expected origin type got something else"
            | NewDestMerdEntry m, DestMerd d -> handleNewDestMerdEntry d m
            | NewDestMerdEntry m, _ -> failwithf "expected dest type got something else"
        {model with form = form'}, Cmd.none
    
    let handleNewOriginMerdEntry (formOpt : OriginMerdEntry option) (msg : NewOriginMerdEntry) =
        let form = formOpt |> Option.defaultValue OriginMerd.New
        let result =
            match msg with
            | AddOriginMerdID i -> {form with originMerdID = i}
            | AddMerdNumber n -> {form with merdNumber = n}
            | AddPD p -> {form with pD = p}
            | AddAverageWeight w -> {form with averageWeight = w}
        OriginMerd (Some result)
    
    let handleNewDestMerdEntry (formOpt : DestMerdEntry option) (msg : NewDestMerdEntry) =
        let form = formOpt |> Option.defaultValue DestMerd.New
        let result =
            match msg with
            | AddDestMerdID i -> {form with destMerdID = i}
        DestMerd (Some result)
    

    Any time you say, "Hey, there's a lot of repetition here", there's usually a way to extract it to a common function. The F# type system is your friend here: when you're hip-deep in a refactoring like this, you won't always remember what functions you've changed already and which ones you haven't. Just look for the red wavy lines, though, and you'll know which functions you still need to work on. Hopefully this example will inspire you to spot other common code that can be extracted to its own function.