Remove duplication about safe-bookstore

[et1975]

I don't know if there's a good answer for what you asking, but a couple of small observations:

• You should never need to call urlUpdate from update
• Delegate as much as possible into child modules (you can think of auth as viewless child)

[forki]

• regarding urlupdate - care to send one of your famous refactoring PRs?
• yes but even if you push more into the submodules. it's still that weird pattern match - but on the other hand it's super explicit. Much better than hiding such things in DI containers. and compiler tells me if I forget. so ...

[bentayloruk]

This match also looks funky to me, because when we have a message for a page, but we don't have the correct sub-model for that page (in the app model), we just drop through (i.e. ignore the message). Is this considered OK, from a logical perspective? It feels like it should be an error, but I'm assuming there must be conditions where you could get in this state legitimately.

[bentayloruk]

@forki do you think it would improve things, if we promote the SubModel to the top level match? To me, this makes it clearer and makes the drop through case distinct too.

let update msg model =
    match msg, model.SubModel with
    | AppMsg.OpenLogIn, _ ->
        let m,cmd = Login.init None
        { model with
            Page = Page.Login
            SubModel = LoginModel m }, Cmd.batch [cmd; Navigation.modifyUrl (toHash Page.Login) ]

    | StorageFailure e, _ ->
        printfn "Unable to access local storage: %A" e
        model, []

    | LoginMsg msg, LoginModel m ->
        let m,cmd = Login.update msg m
        let cmd = Cmd.map LoginMsg cmd
        match m.State with
        | Login.LoginState.LoggedIn token ->
            let newUser : UserData = { UserName = m.Login.UserName; Token = token }
            let cmd =
                if model.Menu.User = Some newUser then cmd else
                Cmd.batch [cmd
                           Cmd.ofFunc (Utils.save "user") newUser (fun _ -> LoggedIn) StorageFailure ]

            { model with
                SubModel = LoginModel m
                Menu = { model.Menu with User = Some newUser }}, cmd
        | _ ->
            { model with
                SubModel = LoginModel m
                Menu = { model.Menu with User = None } }, cmd

    | LoginMsg msg, _ -> model, Cmd.none

    | PromoMsg msg, PromoModel m ->
        let m, cmd = Promo.update msg m
        let cmd = Cmd.map PromoMsg cmd
        { model with 
            SubModel = PromoModel m }, cmd

    | PromoMsg msg, _ -> model, Cmd.none
        
    | WishListMsg msg, WishListModel m ->
        let m,cmd = WishList.update msg m
        let cmd = Cmd.map WishListMsg cmd
        { model with
            SubModel = WishListModel m }, cmd
    
    | WishListMsg msg, _ -> model, Cmd.none

    | AppMsg.LoggedIn, _ ->
        let nextPage = Page.WishList
        let m,cmd = urlUpdate (Some nextPage) model
        match m.Menu.User with
        | Some user ->
            m, Cmd.batch [cmd; Navigation.modifyUrl (toHash nextPage) ]
        | None ->
            m, Cmd.ofMsg Logout

    | AppMsg.LoggedOut, _ ->
        { model with
            Page = Page.Home
            SubModel = NoSubModel
            Menu = { model.Menu with User = None } },
        Navigation.modifyUrl (toHash Page.Home)

    | AppMsg.Logout, _ ->
        model, Cmd.ofFunc Utils.delete "user" (fun _ -> LoggedOut) StorageFailure

[forki]

yes. thought I already did that

[bentayloruk]

The code you link to above, in master has the nested match. Want me to PR it?

[forki]

Yes please send a pull request Am 20.06.2017 1:11 nachm. schrieb "Ben Taylor" <[email protected]>:

…

The code you link to above, in master has the nested match. Want me to PR it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#97 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNLpENZwNzvT7EJQN187x8ajedIgiks5sF6j2gaJpZM4M7rTj> .