Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do all update logic in one place #3

Open
pettermahlen opened this issue Apr 13, 2018 · 0 comments
Open

Do all update logic in one place #3

pettermahlen opened this issue Apr 13, 2018 · 0 comments

Comments

@pettermahlen
Copy link

In the Update function, there are a couple of cases like this:

is PlayButtonClicked ->
            onPlayButtonClicked(model.copy(musicLocation = event.musicLocation), event.context)
...
fun onPlayButtonClicked(model: MainModel, context: Context): Next<MainModel, MainEffect> = Next.next(model,
        effects(AttemptToPlay(model.musicLocation, model.status, context)))

It's probably easier to read if all the business logic for handling the event is in a single place rather than two, like so:

is PlayButtonClicked ->
            onPlayButtonClicked(model, event)
...
fun onPlayButtonClicked(model: MainModel, event: PlayButtonClicked): Next<MainModel, MainEffect> =
   Next.next(model.copy(musicLocation = event.musicLocation),
             effects(AttemptToPlay(event.musicLocation, model.status, event.context)))

Some Spotify teams consistently use functions with the signature onEventName(model, event) for all event updates, even if the actual update doesn't need any data from the model/event. I'm personally leaning towards inlining simple updates and only passing relevant information, so something like:

return when (event) {
   is SomethingSimple -> Next.noChange()
   is NeedsDataFromEventOnly -> onNDFO(event)
   is NeedsModel -> onNeedsModel(model)
   // ...
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant