Search code examples
androidunit-testingmvpandroid-mvp

Is a bad practice passing references of a View to a Presenter in MVP Pattern?


I have a big project for Android with Kotlin using the MVP pattern, and I'm starting to struggle to do Unit Tests (testing the presenters mocking the view interfaces). The reason is I'm passing view references to my functions in the presenters and it's really bad having to mock them, example:

My code would look like:

class MainActivity : Activity(), MainActivityView {

    @BindView(R.id.numberTV)
    lateinit var numberTV : AppCompatTextView

    private val mainActivityPresenter = MainActivityPresenter(this)

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)

        mainActivityPresenter.onCreate()
    }

    override fun showNumber() {
        mainActivityPresenter.showNumber(numberTV, 22)
    }

}

interface MainActivityView {
    fun showNumber()
}

class MainActivityPresenter(private val mainActivityView: MainActivityView) {
    fun showNumber(numberTV: AppCompatTextView, number: Int) {
        numberTV.text = if (number < 0) {
            "Not compatible"
        } else if (number < 10) {
            number.toString()
        } else {
            "9+"
        }
    }

    fun onCreate() {
        mainActivityView.showNumber()
    }
}

My current problem is, when I'm testing the function showNumber(AppCompatTextView, Int) with Mockito in Unit Tests, I should mock the view just to pass the test (as it can't be null).

Which one would be a better approach to do Unit Tests here?

My thoughts are:

  1. Grabbing the numberTV: AppCompatTextView from the MainActivityPresenter, such as mainActivityPresenter.getBindViews().mainIV
  2. Returning just the text logic ("Not compatible", number.toString() or "9+") from the presenter, although sometimes the requirements require to do logic between more than view (2+)

What would you do?


EDIT

I would like to point out that not passing any View to the presenter, might be an issue with asynchronous calls.

Example: setting an image with Glide:

internal fun showImageAccordingToCache(cachedSplashScreenUri: String?, mainImageView: ImageView) {
    Glide.with(context)
            /**
             * Save in cache, but we say to load it from cache. Otherwise it will throw an error
             */
            .setDefaultRequestOptions(defaultDiskStrategy()
                    .onlyRetrieveFromCache(true))
            .load(cachedSplashScreenUri)
            .listener(object : RequestListener<Drawable> {
                override fun onLoadFailed(e: GlideException?, model: Any?, target: Target<Drawable>?, isFirstResource: Boolean): Boolean {
                    /**
                     * And when that error is thrown, we preload the image for the next time.
                     */
                    activityViewPresenter.showLogo()
                    loadImageInCache(cachedSplashScreenUri)
                    return true
                }

                override fun onResourceReady(resource: Drawable?, model: Any?, target: Target<Drawable>?, dataSource: DataSource?, isFirstResource: Boolean): Boolean {
                    return false
                }
            })
            .into(mainImageView)
}

Solution

  • It's normal to have an interface View in your presenter. In your case MainActivityView is the interface that represents the contract your Activity must comply to. You'd normally pass that view in the constructor of the presenter (what you're already doing) on by using dagger to inject it into the presenter.

    Now this, is not very usual:

     fun showNumber(numberTV: AppCompatTextView, number: Int) {
            numberTV.text = if (number < 0) {
                "Not compatible"
            } else if (number < 10) {
                number.toString()
            } else {
                "9+"
            }
        }
    

    Now the presenter knows about "Android SDK components" which is not a good thing. In this case what you should be doing is this:

     fun showNumber(number: Int) {
            if (number < 0) {
                mainActivityView.setNumberText("Not compatible");
            } else if (number < 10) {
                mainActivityView.setNumberText(number.toString());
            } else {
                mainActivityView.setNumberText("9+");
            }
        }
    

    To test this you would mock the view and see if depending on number each of those methods were actually called.(In java).

    @Mock
    MainActivityView view;
    
    @Test
    public fun shouldShowCorrectNumber() {
    
        int number = 10;
        presenter.showNumber(number);
        verify(view).showNumber("9+");
    }
    

    As for async calls, what I usually see when using the Glide library is that it is used in the activity, not in the presenter. Other types of async calls might go in other layers, for example. I usually see network calls in the Interactor layer with callbacks to the presenter: