search

GCX-HCI / ThirtyInch

a MVP library for Android favoring a stateful Presenter
Apache License 2.0
1.03k stars 101 forks source link

Add CoroutineScope for TiPresenter #182

Closed Syex closed 5 years ago

Syex commented 5 years ago

This PR adds a new module for Kotlin's coroutines which contains a custom CoroutineScope to be used when starting coroutines in a TiPresenter. The so started coroutines are then bound to the lifecycle of the presenter.

Mostly it will behave similar to the RxTiPresenterDisposableHandler introduced in #161, but instead of having to add disposeWhenDestroyed or disposeWhenViewDetached the TiCoroutineScope will cancel jobs automatically when the presenter reaches the specific lifecycle state.

If there's a better workaround to saving the last presenterState in a variable please let me know, maybe I just misunderstood the calls 🤷‍♂️

StefMa commented 5 years ago

Kotlim 1.3.20 is already released

On Fri, Jan 25, 2019, 2:52 PM Tom Seifert <notifications@github.com wrote:

This PR adds a new module for Kotlin's coroutines which contains a custom CoroutineScope to be used when starting coroutines in a TiPresenter. The so started coroutines are then bound to the lifecycle of the presenter.

Mostly it will behave similar to the RxTiPresenterDisposableHandler introduced in #161 https://github.com/grandcentrix/ThirtyInch/pull/161, but instead of having to add disposeWhenDestroyed or disposeWhenViewDetached the TiCoroutineScope will cancel jobs automatically when the presenter reaches the specific lifecycle state.

If there's a better workaround to saving the last presenterState in a variable please let me know, maybe I just misunderstood the calls 🤷‍♂️

You can view, comment on, or merge this pull request online at:

https://github.com/grandcentrix/ThirtyInch/pull/182 Commit Summary

  • Add new Kotlin Coroutines module
  • Add TiCoroutineScope and a test
  • Add new module to README

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/grandcentrix/ThirtyInch/pull/182, or mute the thread https://github.com/notifications/unsubscribe-auth/AJwYe3UpthcC4vFtOxJqiqhDgpuNgwjhks5vGwwPgaJpZM4aS6oM .

StefMa commented 5 years ago

The Readme is wrong with the view detaching.. We have to add the boolean "true" als third param.. Beside if that. Does this really require to create each time a new view got attached?

On Fri, Jan 25, 2019, 3:20 PM Stefan M. <stefanmay91@gmail.com wrote:

Kotlim 1.3.20 is already released

On Fri, Jan 25, 2019, 2:52 PM Tom Seifert <notifications@github.com wrote:

This PR adds a new module for Kotlin's coroutines which contains a custom CoroutineScope to be used when starting coroutines in a TiPresenter. The so started coroutines are then bound to the lifecycle of the presenter.

Mostly it will behave similar to the RxTiPresenterDisposableHandler introduced in #161 https://github.com/grandcentrix/ThirtyInch/pull/161, but instead of having to add disposeWhenDestroyed or disposeWhenViewDetached the TiCoroutineScope will cancel jobs automatically when the presenter reaches the specific lifecycle state.

If there's a better workaround to saving the last presenterState in a variable please let me know, maybe I just misunderstood the calls 🤷‍♂️

You can view, comment on, or merge this pull request online at:

https://github.com/grandcentrix/ThirtyInch/pull/182 Commit Summary

  • Add new Kotlin Coroutines module
  • Add TiCoroutineScope and a test
  • Add new module to README

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/grandcentrix/ThirtyInch/pull/182, or mute the thread https://github.com/notifications/unsubscribe-auth/AJwYe3UpthcC4vFtOxJqiqhDgpuNgwjhks5vGwwPgaJpZM4aS6oM .

Syex commented 5 years ago

Beside if that. Does this really require to create each time a new view got attached?

I was prepared for that question! Yes we have to. The CoroutineScope requires its property coroutineContext to be final. We're initializing it with a Job(), which we cancel later. Once it is canceled we can't restart it and we can't reassign the coroutineContext. Do you see any other option?

Syex commented 5 years ago

Changed the implementation. The flag, whether the scope should cancel all jobs when the view detaches, is no longer required to be constructor parameter.

Instead, there's now a method launchUntilViewDetaches on the scope which should be used for this case, rather than the default launch method.