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

Checking state during action execution #45

Closed
wants to merge 1 commit into from
Closed

Checking state during action execution #45

wants to merge 1 commit into from

Conversation

Otacon
Copy link

@Otacon Otacon commented Sep 15, 2020

The state of the UIDataStore should be checked right before running the Action and not when trying to enqueue it.
Let's make a simple test:

Activity

class Activity: Activity{ 
    onCreate() { viewModel.onCreate() }
    onStart() { viewModel.onStart() }
    onResume() { viewModel.onResume() }
    ...
}

ViewModel

class MyViewModel: AndroidDataFlow(){
    ...
    fun onCreate() = actionOn<UIState.Empty>{ setState { MyState.Created } }
    fun onStart() = actionOn<MyState.Created>{ setState { MyState.Started } }
    fun onResume() = actionOn<MyState.Started>{ setState { MyState.Resumed } }
    ...
}

When onCreate is called, the actionOn will be enqueued successfully.
Now onStart will be called, but still the first action hasn't been executed yet, so the current state is still UIState.Empty. This will cause the submission of the onStart action to fail with BadOrWrongState event.

With this fix, the order of the actions will be preserved, since the check is done while running the action.

.collect { dataUpdate ->
uiDataStore.pushNewData(dataUpdate)
if (!action.klass.isInstance(currentState)) {
enqueueAction(ActionFlow(currentState::class as KClass<UIState>, onSuccess = { sendEvent { UIEvent.BadOrWrongState(currentState) } }, onError = action.onError))
Copy link
Author

@Otacon Otacon Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the correct way of sending a UIEvent.BadOrWrongState internally

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, perhaps we can register the state class when we make an action. In case of simple action, no need of particular class. In actionOn case, we need it

@erikhuizinga
Copy link
Collaborator

I'm sorry, I have no time to review this PR this or next week.

@arnaudgiuliani
Copy link
Collaborator

I'm sorry, I have no time to review this PR this or next week.

no worry, sorry for disturbing 👍

@arnaudgiuliani arnaudgiuliani removed the request for review from erikhuizinga September 16, 2020 14:42
@arnaudgiuliani
Copy link
Collaborator

I'm closing this PR as I've made a fix in new engine. Let's keep in touch.

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

Successfully merging this pull request may close these issues.

None yet

4 participants