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

What purpose in setPresenter() method inside View wich has injection of that's presenter #27

Open
mong0ose opened this issue Nov 20, 2017 · 3 comments

Comments

@mong0ose
Copy link

mong0ose commented Nov 20, 2017

override fun setPresenter(presenter: BrowseBufferoosContract.Presenter) {

Hello, I figured that you set presenter in BrowseActivity using dagger injection and method setPresenter() which I think would be redundant becouse of using dagger. Maybe you can correct me, if I'm mistaken.

Also I'm pretty excited about this article with new vision of clean architecture, and your second topic with new architecture components from google is good too. Nice work!

@katien
Copy link

katien commented Dec 21, 2017

I was wondering about this too. I can remove
fun setPresenter(presenter: T) from the BaseView and rely on dependency injection to give the view a presenter reference. I'd like to know the rationale for having setPresenter() instead of nothing or just a val presenter: T which would be initialized by dagger.

Another thing which seemed redundant is the @Inject annotation on the presenter's constructor here:

class BrowseBufferoosPresenter @Inject constructor(
    val browseView: BrowseBufferoosContract.View, 
    val getBufferoosUseCase: SingleUseCase<List<Bufferoo>, Void>, 
    val bufferooMapper: BufferooMapper): BrowseBufferoosContract.Presenter {

We already provide the BrowseBufferoosPresenter in our BrowseActivityModule, so removing this annotation doesn't seem to affect anything. I was thinking that these might just seem redundant in the example app and have some purpose in a larger scale application though. Any insight would be appreciated.

@qwertyfinger
Copy link

setPresenter function is indeed redundant. There are different ways to associate Presenter and View, so maybe it stayed there by mistake.
BaseView is nice to have for a clear hierarchy, even if it doesn't contain anything. And maybe it will contain some other base functions depending on your requirements.

@qwertyfinger
Copy link

@DevKate Regarding @Inject annotation – in the current setting it is also redundant.
But what I like to do is to use it and instead remove the corresponding @Provides function.
There are several reasons for that:

  • @Provides function should be static and in Kotlin it's cumbersome to actually implement this:
@Module
abstract class BrowseActivityModule {

    // Note that @Scope annotation is gone now, we should mark the implementation class
    // (i.e. BrowseActivity) with it, not the interface
    @Binds
    abstract fun provideBrowseView(
        browseActivity: BrowseActivity
    ): BrowseBufferoosContract.View

    @Module companion object {

        @JvmStatic
        @PerActivity
        @Provides
        fun provideBrowsePresenter(
            mainView: BrowseBufferoosContract.View,
            getBufferoos: GetBufferoos,
            mapper: BufferooMapper
        ): BrowseBufferoosContract.Presenter {
            return BrowseBufferoosPresenter(mainView, getBufferoos, mapper)
        }

    }

}
  • If we were to add a new constructor parameter to the Presenter, we would have to duplicate the change in @Provides function each time.
  • With constructor injection we can switch to @Binds functions which should be preferred to @Provides functions.

Now in order to do that in Presenter we need to use concrete Use Case implementation, not the SingleUseCase interface.
It's hard for me to see the case in which we would need to substitute one Use Case interface with different implementations.
And using val getBufferoosUseCase: GetBufferoos instead of val getBufferoosUseCase: SingleUseCase<List<Bufferoo>, Void> seems much less verbose and more clear to me.
But, of course, if for some reason you need to use interfaces, then you have to stick with @Provides functions.
If not, then let's summarise:

  • You get shorter and more comprehensible (IMO) constructor parameter declarations for Presenter.
  • You get more concise, more readable and perhaps even slightly more performant @Module class implementation:
@Module
abstract class BrowseActivityModule {

    @Binds
    abstract fun provideBrowseView(
        browseActivity: BrowseActivity
    ): BrowseBufferoosContract.View

    @Binds
    abstract fun provideBrowsePresenter(
        browseBufferoosPresenter: BrowseBufferoosPresenter
    ) : BrowseBufferoosContract.Presenter

}

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

3 participants