GCX-HCI / ThirtyInch

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

add extension functions for boilerplate-less Disposable handling #161

Closed lukaszkalnik closed 5 years ago

lukaszkalnik commented 5 years ago

Create Kotlin helper interface DisposableHandler to be implemented by TiPresenter to provide syntactic sugar for Disposable handling.

Implementing this interface by your Presenter enables you to call .disposeWhenDestroyed() and .disposeWhenViewDetached() as extension functions directly on your Disposable instances.

Usage:

 class MyPresenter : TiPresenter<MyView>(), DisposableHandler {

    // configure with reference to your Presenter instance
     override val disposableHandler = RxTiPresenterDisposableHandler(this)

     override fun onCreate() {
         super.onCreate()

         // Presenter lifecycle dependent Disposable
         myObservable
             .subscribe()
             .disposeWhenDestroyed()
     }

     override fun onAttachView(view: MyView) {
         super.onAttachView(view)

         // View attached/detached dependent Disposable
         myViewObservable
             .subscribe()
             .disposeWhenViewDetached()
     }
 }
Syex commented 5 years ago

What about for convenience adding a subclass of TiPresenter that implements the DisposableHandler for me? I'm sure we mostly have this class in our projects already and it would be the first thing I'd do in a project, because it adds that extra interface implementation and override as noise is every presenter otherwise.

The overhead when not needing the disposableHandler would be less important to me than having to implement and override this every time.

lukaszkalnik commented 5 years ago

@Syex it's a good idea, although this interface was specifically meant as composition over inheritance, to not require a BasePresenter class to be extended by every Presenter. But I will provide a convenience implementation of it.

jreehuis commented 5 years ago

For our projects this really is a big win. But I do not like that we will include RxJava in each project which uses the Kotlin module. I guess there are projects which do not want to use RxJava but the Kotlin module.

lukaszkalnik commented 5 years ago

@jreehuis then maybe we should put it in the thirtyinch-rx2 module? This is probably semantically more fitting, because indeed as you noticed it is related only to Rx usage in ThirtyInch.

jreehuis commented 5 years ago

@lukaszkalnik Than you get the issue that maybe someone want to use Rx but not Kotlin. But I guess that way around would be better fitting. What does the others think?

lukaszkalnik commented 5 years ago

I think most Android projects are moving to Kotlin sooner or later.

lukaszkalnik commented 5 years ago

@passsy (and others), do we want this added or not? If we don't want to add Kotlin support to ThirtyInch just for this small helper I think we should make a decision and close the PR. I'm fine with that.

lukaszkalnik commented 5 years ago

I think the best solution would be to just publish it as a gist and link to it in the README.

lukaszkalnik commented 5 years ago

Link to gist: https://gist.github.com/lukaszkalnik/d9fb755021b385672fa4cfe29f6ccf81